Changes lifetime of ApplicationConnection instances.

Before this change, ApplicationConnections were owned by the ApplicationImpl, one created for every call to ConnectToApplication/AcceptConnection. While this makes sense for incoming connections (it doesn't make sense to transfer ownership of the connection to the delegate), it doesn't for outgoing connections.

So we change ConnectToApplication to return a scoped_ptr<ApplicationConnection> which the caller must manage.

R=sky@chromium.org
http://crbug.com/519583

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

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

30 files changed, 125 insertions, 214 deletions

diff --git a/components/html_viewer/ax_provider_apptest.cc b/components/html_viewer/ax_provider_apptest.cc
index 2b77334..073926c 100644
--- a/components/html_viewer/ax_provider_apptest.cc
+++ b/components/html_viewer/ax_provider_apptest.cc
@@ -82,8 +82,8 @@ TEST_F(AXProviderTest, HelloWorld) {
   mojo::URLRequestPtr request(mojo::URLRequest::New());
   request->url = mojo::String::From(
       base::StringPrintf("http://127.0.0.1:%u/files/test.html", assigned_port));
-  ApplicationConnection* connection = application_impl()->ConnectToApplication(
-      request.Pass());
+  scoped_ptr<ApplicationConnection> connection =
+      application_impl()->ConnectToApplication(request.Pass());
 
   // Embed the html_viewer in a View.
   ViewManagerClientPtr view_manager_client;
diff --git a/components/html_viewer/blink_platform_impl.cc b/components/html_viewer/blink_platform_impl.cc
index a1a988a..083b5c3 100644
--- a/components/html_viewer/blink_platform_impl.cc
+++ b/components/html_viewer/blink_platform_impl.cc
@@ -74,7 +74,7 @@ BlinkPlatformImpl::BlinkPlatformImpl(
   if (app) {
     mojo::URLRequestPtr request(mojo::URLRequest::New());
     request->url = mojo::String::From("mojo:network_service");
-    mojo::ApplicationConnection* connection =
+    scoped_ptr<mojo::ApplicationConnection> connection =
         app->ConnectToApplication(request.Pass());
     connection->ConnectToService(&network_service_);
     connection->ConnectToService(&url_loader_factory_);
diff --git a/components/html_viewer/html_document_application_delegate.cc b/components/html_viewer/html_document_application_delegate.cc
index 68eff9f..935f5ee 100644
--- a/components/html_viewer/html_document_application_delegate.cc
+++ b/components/html_viewer/html_document_application_delegate.cc
@@ -115,7 +115,7 @@ void HTMLDocumentApplicationDelegate::OnTerminate() {
 void HTMLDocumentApplicationDelegate::Initialize(mojo::ApplicationImpl* app) {
   mojo::URLRequestPtr request(mojo::URLRequest::New());
   request->url = mojo::String::From("mojo:network_service");
-  mojo::ApplicationConnection* connection =
+  scoped_ptr<mojo::ApplicationConnection> connection =
       app_.ConnectToApplication(request.Pass());
   connection->ConnectToService(&network_service_);
   connection->ConnectToService(&url_loader_factory_);
diff --git a/components/view_manager/public/cpp/lib/view_manager_init.cc b/components/view_manager/public/cpp/lib/view_manager_init.cc
index 0a540e9..21fd341 100644
--- a/components/view_manager/public/cpp/lib/view_manager_init.cc
+++ b/components/view_manager/public/cpp/lib/view_manager_init.cc
@@ -52,9 +52,7 @@ ViewManagerInit::ViewManagerInit(ApplicationImpl* app,
   }
 }
 
-ViewManagerInit::~ViewManagerInit() {
-  connection_->CloseConnection();
-}
+ViewManagerInit::~ViewManagerInit() {}
 
 void ViewManagerInit::OnCreate(InterfaceRequest<ViewManagerClient> request) {
   // TODO(sky): straighten out lifetime.
diff --git a/components/view_manager/public/cpp/view_manager_init.h b/components/view_manager/public/cpp/view_manager_init.h
index 3d5129d..347eb94 100644
--- a/components/view_manager/public/cpp/view_manager_init.h
+++ b/components/view_manager/public/cpp/view_manager_init.h
@@ -33,7 +33,7 @@ class ViewManagerInit {
   ViewManagerRoot* view_manager_root() { return view_manager_root_.get(); }
 
   // Returns the application connection established with the view manager.
-  ApplicationConnection* connection() { return connection_; }
+  ApplicationConnection* connection() { return connection_.get(); }
 
  private:
   class ClientFactory;
@@ -41,7 +41,7 @@ class ViewManagerInit {
   void OnCreate(InterfaceRequest<ViewManagerClient> request);
 
   ApplicationImpl* app_;
-  ApplicationConnection* connection_;
+  scoped_ptr<ApplicationConnection> connection_;
   ViewManagerDelegate* delegate_;
   scoped_ptr<ClientFactory> client_factory_;
   ViewManagerRootPtr view_manager_root_;
diff --git a/components/view_manager/view_manager_client_apptest.cc b/components/view_manager/view_manager_client_apptest.cc
index 960625e..0d5f4e0 100644
--- a/components/view_manager/view_manager_client_apptest.cc
+++ b/components/view_manager/view_manager_client_apptest.cc
@@ -184,7 +184,7 @@ class ViewManagerTest : public ViewManagerTestBase {
   void ConnectToApplicationAndEmbed(View* view) {
     mojo::URLRequestPtr request(mojo::URLRequest::New());
     request->url = mojo::String::From(application_impl()->url());
-    ApplicationConnection* connection =
+    scoped_ptr<ApplicationConnection> connection =
         application_impl()->ConnectToApplication(request.Pass());
     mojo::ViewManagerClientPtr client;
     connection->ConnectToService(&client);
@@ -197,7 +197,7 @@ class ViewManagerTest : public ViewManagerTestBase {
                             mojo::ViewManagerClientPtr* client) override {
     on_will_embed_count_++;
     if (on_will_embed_return_value_) {
-      ApplicationConnection* connection =
+      scoped_ptr<ApplicationConnection> connection =
           application_impl()->ConnectToApplication(request.Pass());
       connection->ConnectToService(client);
     } else {
diff --git a/components/view_manager/view_manager_service_apptest.cc b/components/view_manager/view_manager_service_apptest.cc
index e18c23d..bc3db9b 100644
--- a/components/view_manager/view_manager_service_apptest.cc
+++ b/components/view_manager/view_manager_service_apptest.cc
@@ -78,7 +78,7 @@ bool EmbedUrl(mojo::ApplicationImpl* app,
   {
     mojo::URLRequestPtr request(mojo::URLRequest::New());
     request->url = mojo::String::From(url);
-    ApplicationConnection* connection =
+    scoped_ptr<ApplicationConnection> connection =
         app->ConnectToApplication(request.Pass());
     mojo::ViewManagerClientPtr client;
     connection->ConnectToService(&client);
@@ -343,7 +343,7 @@ class ViewManagerClientImpl : public mojo::ViewManagerClient,
       const OnEmbedForDescendantCallback& callback) override {
     tracker()->OnEmbedForDescendant(view);
     mojo::ViewManagerClientPtr client;
-    ApplicationConnection* connection =
+    scoped_ptr<ApplicationConnection> connection =
         app_->ConnectToApplication(request.Pass());
     connection->ConnectToService(&client);
     callback.Run(client.Pass());
@@ -562,7 +562,7 @@ class ViewManagerServiceAppTest : public mojo::test::ApplicationTestBase,
     client_factory_.reset(new ViewManagerClientFactory(application_impl()));
     mojo::URLRequestPtr request(mojo::URLRequest::New());
     request->url = mojo::String::From("mojo:view_manager");
-    ApplicationConnection* vm_connection =
+    scoped_ptr<ApplicationConnection> vm_connection =
         application_impl()->ConnectToApplication(request.Pass());
     vm_connection->AddService(client_factory_.get());
     vm_connection->ConnectToService(&view_manager_root_);
diff --git a/mandoline/tab/frame_connection.h b/mandoline/tab/frame_connection.h
index 2aa3558..63065e9 100644
--- a/mandoline/tab/frame_connection.h
+++ b/mandoline/tab/frame_connection.h
@@ -6,6 +6,7 @@
 #define MANDOLINE_TAB_FRAME_CONNECTION_H_
 
 #include "base/basictypes.h"
+#include "base/memory/scoped_ptr.h"
 #include "components/view_manager/public/interfaces/view_manager.mojom.h"
 #include "mandoline/tab/frame_user_data.h"
 #include "mandoline/tab/public/interfaces/frame_tree.mojom.h"
@@ -33,14 +34,14 @@ class FrameConnection : public FrameUserData {
   FrameTreeClient* frame_tree_client() { return frame_tree_client_.get(); }
 
   mojo::ApplicationConnection* application_connection() {
-    return application_connection_;
+    return application_connection_.get();
   }
 
  private:
   FrameTreeClientPtr frame_tree_client_;
 
   // TODO(sky): needs to be destroyed when connection lost.
-  mojo::ApplicationConnection* application_connection_;
+  scoped_ptr<mojo::ApplicationConnection> application_connection_;
 
   DISALLOW_COPY_AND_ASSIGN(FrameConnection);
 };
diff --git a/mandoline/ui/browser/browser.cc b/mandoline/ui/browser/browser.cc
index 15ab1f2..c8cdac3 100644
--- a/mandoline/ui/browser/browser.cc
+++ b/mandoline/ui/browser/browser.cc
@@ -117,7 +117,7 @@ void Browser::OnEmbedForDescendant(mojo::View* view,
   if (!frame || !frame->HasAncestor(frame_tree_->root())) {
     // TODO(sky): add requestor url so that we can return false if it's not
     // an app we expect.
-    mojo::ApplicationConnection* connection =
+    scoped_ptr<mojo::ApplicationConnection> connection =
         app_->ConnectToApplication(request.Pass());
     connection->ConnectToService(client);
     return;
@@ -215,7 +215,7 @@ void Browser::ShowOmnibox(mojo::URLRequestPtr request) {
     omnibox_->SetBounds(root_->bounds());
   }
   mojo::ViewManagerClientPtr view_manager_client;
-  mojo::ApplicationConnection* connection =
+  scoped_ptr<mojo::ApplicationConnection> connection =
       app_->ConnectToApplication(request.Pass());
   connection->AddService<ViewEmbedder>(this);
   connection->ConnectToService(&view_manager_client);
diff --git a/mandoline/ui/browser/browser_apptest.cc b/mandoline/ui/browser/browser_apptest.cc
index f99a842..51815b9 100644
--- a/mandoline/ui/browser/browser_apptest.cc
+++ b/mandoline/ui/browser/browser_apptest.cc
@@ -52,7 +52,6 @@ class BrowserTest : public mojo::test::ApplicationTestBase,
  public:
   BrowserTest()
       : app_(nullptr),
-        last_closed_connection_(nullptr),
         last_browser_closed_(nullptr) {}
 
   // Creates a new Browser object.
@@ -75,21 +74,11 @@ class BrowserTest : public mojo::test::ApplicationTestBase,
     return last_browser;
   }
 
-  // Returns the last ApplicationConnection closed.
-  void* last_closed_connection() {
-    return last_closed_connection_;
-  }
-
   // Overridden from ApplicationDelegate:
   void Initialize(mojo::ApplicationImpl* app) override {
     app_ = app;
   }
 
-  void OnWillCloseConnection(mojo::ApplicationConnection* connection) override {
-    // WARNING: DO NOT FOLLOW THIS POINTER. IT WILL BE DESTROYED.
-    last_closed_connection_ = connection;
-  }
-
   // ApplicationTestBase:
   ApplicationDelegate* GetApplicationDelegate() override { return this; }
 
@@ -110,7 +99,6 @@ class BrowserTest : public mojo::test::ApplicationTestBase,
 
  private:
   mojo::ApplicationImpl* app_;
-  void* last_closed_connection_;
   std::set<TestBrowser*> browsers_;
   TestBrowser* last_browser_closed_;
   scoped_ptr<base::RunLoop> browser_closed_run_loop_;
@@ -125,9 +113,13 @@ TEST_F(BrowserTest, ClosingBrowserClosesAppConnection) {
   ASSERT_NE(nullptr, browser);
   mojo::ApplicationConnection* view_manager_connection =
       browser->view_manager_init_.connection();
+  mojo::ApplicationConnection::TestApi connection_test_api(
+      view_manager_connection);
   ASSERT_NE(nullptr, view_manager_connection);
+  base::WeakPtr<mojo::ApplicationConnection> ptr =
+      connection_test_api.GetWeakPtr();
   BrowserClosed(browser);
-  EXPECT_EQ(last_closed_connection(), view_manager_connection);
+  EXPECT_FALSE(ptr);
 }
 
 // This test verifies that we can create two Browsers and each Browser has a
diff --git a/mandoline/ui/browser/desktop/desktop_ui.cc b/mandoline/ui/browser/desktop/desktop_ui.cc
index 889ee64..2e5737c 100644
--- a/mandoline/ui/browser/desktop/desktop_ui.cc
+++ b/mandoline/ui/browser/desktop/desktop_ui.cc
@@ -151,10 +151,10 @@ void DesktopUI::ButtonPressed(views::Button* sender, const ui::Event& event) {
     DCHECK(!client_binding_.is_bound());
     mojo::URLRequestPtr request(mojo::URLRequest::New());
     request->url = mojo::String::From("mojo:omnibox");
-    mojo::ApplicationConnection* application_connection =
+    omnibox_connection_ =
         application_impl_->ConnectToApplication(request.Pass());
-    application_connection->AddService<ViewEmbedder>(browser_);
-    application_connection->ConnectToService(&omnibox_);
+    omnibox_connection_->AddService<ViewEmbedder>(browser_);
+    omnibox_connection_->ConnectToService(&omnibox_);
     OmniboxClientPtr client;
     client_binding_.Bind(&client);
     omnibox_->SetClient(client.Pass());
diff --git a/mandoline/ui/browser/desktop/desktop_ui.h b/mandoline/ui/browser/desktop/desktop_ui.h
index 90ed638..6f5ce3f 100644
--- a/mandoline/ui/browser/desktop/desktop_ui.h
+++ b/mandoline/ui/browser/desktop/desktop_ui.h
@@ -11,6 +11,7 @@
 #include "ui/views/layout/layout_manager.h"
 
 namespace mojo {
+class ApplicationConnection;
 class Shell;
 class View;
 }
@@ -53,6 +54,7 @@ class DesktopUI : public BrowserUI,
   mojo::View* content_;
   OmniboxPtr omnibox_;
   mojo::Binding<OmniboxClient> client_binding_;
+  scoped_ptr<mojo::ApplicationConnection> omnibox_connection_;
 
   DISALLOW_COPY_AND_ASSIGN(DesktopUI);
 };
diff --git a/media/mojo/services/media_apptest.cc b/media/mojo/services/media_apptest.cc
index 4d08fba..958c662 100644
--- a/media/mojo/services/media_apptest.cc
+++ b/media/mojo/services/media_apptest.cc
@@ -59,12 +59,11 @@ class MediaAppTest : public mojo::test::ApplicationTestBase {
 
     mojo::URLRequestPtr request = mojo::URLRequest::New();
     request->url = "mojo:media";
-    mojo::ApplicationConnection* connection =
-        application_impl()->ConnectToApplication(request.Pass());
-    connection->SetRemoteServiceProviderConnectionErrorHandler(
+    connection_ = application_impl()->ConnectToApplication(request.Pass());
+    connection_->SetRemoteServiceProviderConnectionErrorHandler(
         base::Bind(&MediaAppTest::ConnectionClosed, base::Unretained(this)));
 
-    connection->ConnectToService(&service_factory_);
+    connection_->ConnectToService(&service_factory_);
     service_factory_->CreateCdm(mojo::GetProxy(&cdm_));
     service_factory_->CreateRenderer(mojo::GetProxy(&renderer_));
 
@@ -122,6 +121,8 @@ class MediaAppTest : public mojo::test::ApplicationTestBase {
   StrictMock<MockDemuxerStream> video_demuxer_stream_;
 
  private:
+  scoped_ptr<mojo::ApplicationConnection> connection_;
+
   DISALLOW_COPY_AND_ASSIGN(MediaAppTest);
 };
 
diff --git a/mojo/application/public/cpp/BUILD.gn b/mojo/application/public/cpp/BUILD.gn
index 0181322..49338f1 100644
--- a/mojo/application/public/cpp/BUILD.gn
+++ b/mojo/application/public/cpp/BUILD.gn
@@ -29,7 +29,6 @@ source_set("sources") {
     "interface_factory.h",
     "interface_factory_impl.h",
     "lib/app_lifetime_helper.cc",
-    "lib/application_connection.cc",
     "lib/application_delegate.cc",
     "lib/application_impl.cc",
     "lib/application_runner.cc",
diff --git a/mojo/application/public/cpp/application_connection.h b/mojo/application/public/cpp/application_connection.h
index 5fe14ef..e4affba 100644
--- a/mojo/application/public/cpp/application_connection.h
+++ b/mojo/application/public/cpp/application_connection.h
@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "base/memory/weak_ptr.h"
 #include "mojo/application/public/cpp/lib/interface_factory_connector.h"
 #include "mojo/application/public/interfaces/service_provider.mojom.h"
 
@@ -45,11 +46,20 @@ class ServiceConnector;
 // close a connection, call CloseConnection which will destroy this object.
 class ApplicationConnection {
  public:
-  ApplicationConnection();
+  virtual ~ApplicationConnection() {}
 
-  // Closes the connection and destroys this object. This is the only valid way
-  // to destroy this object.
-  void CloseConnection();
+  class TestApi {
+   public:
+    explicit TestApi(ApplicationConnection* connection)
+        : connection_(connection) {
+    }
+    base::WeakPtr<ApplicationConnection> GetWeakPtr() {
+      return connection_->GetWeakPtr();
+    }
+
+   private:
+    ApplicationConnection* connection_;
+  };
 
   // See class description for details.
   virtual void SetServiceConnector(ServiceConnector* connector) = 0;
@@ -107,21 +117,12 @@ class ApplicationConnection {
       const Closure& handler) = 0;
 
  protected:
-  virtual ~ApplicationConnection();
-
-  // Called to give the derived type to perform some cleanup before destruction.
-  virtual void OnCloseConnection() = 0;
-
- private:
   // Returns true if the connector was set, false if it was not set (e.g. by
   // some filtering policy preventing this interface from being exposed).
   virtual bool SetServiceConnectorForName(ServiceConnector* service_connector,
                                           const std::string& name) = 0;
 
-  // Ensures that CloseConnection can only be called once and the
-  // ApplicationConnection's destructor can only be called after the connection
-  // is closed.
-  bool connection_closed_;
+  virtual base::WeakPtr<ApplicationConnection> GetWeakPtr() = 0;
 };
 
 }  // namespace mojo
diff --git a/mojo/application/public/cpp/application_delegate.h b/mojo/application/public/cpp/application_delegate.h
index 876ad40..a378490 100644
--- a/mojo/application/public/cpp/application_delegate.h
+++ b/mojo/application/public/cpp/application_delegate.h
@@ -34,10 +34,6 @@ class ApplicationDelegate {
   // Return false to reject the connection entirely.
   virtual bool ConfigureOutgoingConnection(ApplicationConnection* connection);
 
-  // Called when an ApplicationConnection is about to close. After returning,
-  // the |connection| object will be destroyed.
-  virtual void OnWillCloseConnection(ApplicationConnection* connection) {}
-
   // Called when the shell connection has a connection error.
   //
   // Return true to shutdown the application. Return false to skip shutting
diff --git a/mojo/application/public/cpp/application_impl.h b/mojo/application/public/cpp/application_impl.h
index 5d2eb45..e05e741 100644
--- a/mojo/application/public/cpp/application_impl.h
+++ b/mojo/application/public/cpp/application_impl.h
@@ -7,6 +7,7 @@
 
 #include <vector>
 
+#include "base/memory/scoped_vector.h"
 #include "base/memory/weak_ptr.h"
 #include "mojo/application/public/cpp/app_lifetime_helper.h"
 #include "mojo/application/public/cpp/application_connection.h"
@@ -78,26 +79,21 @@ class ApplicationImpl : public Application {
 
   // Requests a new connection to an application. Returns a pointer to the
   // connection if the connection is permitted by this application's delegate,
-  // or nullptr otherwise. Caller does not take ownership. The pointer remains
-  // valid until an error occurs on the connection with the Shell, until the
-  // ApplicationImpl is destroyed, or until the connection is closed through a
-  // call to ApplicationConnection::CloseConnection.
+  // or nullptr otherwise. Caller takes ownership.
   // TODO(beng): consider replacing default value in a separate CL per style
   //             guide.
-  ApplicationConnection* ConnectToApplication(
+  scoped_ptr<ApplicationConnection> ConnectToApplication(
       URLRequestPtr request,
       CapabilityFilterPtr filter = nullptr);
 
-  // Closes the |connection|.
-  void CloseConnection(ApplicationConnection* connection);
-
   // Connect to application identified by |request->url| and connect to the
   // service implementation of the interface identified by |Interface|.
   template <typename Interface>
   void ConnectToService(mojo::URLRequestPtr request,
                         InterfacePtr<Interface>* ptr) {
-    ApplicationConnection* connection = ConnectToApplication(request.Pass());
-    if (!connection)
+    scoped_ptr<ApplicationConnection> connection =
+        ConnectToApplication(request.Pass());
+    if (!connection.get())
       return;
     connection->ConnectToService(ptr);
   }
@@ -129,16 +125,13 @@ class ApplicationImpl : public Application {
 
   void OnConnectionError();
 
-  void ClearConnections();
-
   // Called from Quit() when there is no Shell connection, or asynchronously
   // from Quit() once the Shell has OK'ed shutdown.
   void QuitNow();
 
-  typedef std::vector<internal::ServiceRegistry*> ServiceRegistryList;
-
-  ServiceRegistryList incoming_service_registries_;
-  ServiceRegistryList outgoing_service_registries_;
+  // We track the lifetime of incoming connection registries as it more
+  // convenient for the client.
+  ScopedVector<ApplicationConnection> incoming_connections_;
   ApplicationDelegate* delegate_;
   Binding<Application> binding_;
   ShellPtr shell_;
@@ -146,7 +139,6 @@ class ApplicationImpl : public Application {
   Closure termination_closure_;
   AppLifetimeHelper app_lifetime_helper_;
   bool quit_requested_;
-  bool in_destructor_;
   base::WeakPtrFactory<ApplicationImpl> weak_factory_;
 
   MOJO_DISALLOW_COPY_AND_ASSIGN(ApplicationImpl);
diff --git a/mojo/application/public/cpp/lib/application_connection.cc b/mojo/application/public/cpp/lib/application_connection.cc
deleted file mode 100644
index c286c85..0000000
--- a/mojo/application/public/cpp/lib/application_connection.cc
+++ /dev/null
@@ -1,28 +0,0 @@
-// Copyright 2014 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 "mojo/application/public/cpp/application_connection.h"
-
-#include "base/logging.h"
-
-namespace mojo {
-
-ApplicationConnection::ApplicationConnection() : connection_closed_(false) {
-}
-
-void ApplicationConnection::CloseConnection() {
-  if (connection_closed_)
-    return;
-  OnCloseConnection();
-  connection_closed_ = true;
-  delete this;
-}
-
-ApplicationConnection::~ApplicationConnection() {
-  // If this DCHECK fails then something has tried to delete this object without
-  // calling CloseConnection.
-  DCHECK(connection_closed_);
-}
-
-}  // namespace mojo
diff --git a/mojo/application/public/cpp/lib/application_impl.cc b/mojo/application/public/cpp/lib/application_impl.cc
index f24429b..9f2c419 100644
--- a/mojo/application/public/cpp/lib/application_impl.cc
+++ b/mojo/application/public/cpp/lib/application_impl.cc
@@ -52,17 +52,13 @@ ApplicationImpl::ApplicationImpl(ApplicationDelegate* delegate,
       termination_closure_(termination_closure),
       app_lifetime_helper_(this),
       quit_requested_(false),
-      in_destructor_(false),
       weak_factory_(this) {}
 
 ApplicationImpl::~ApplicationImpl() {
-  DCHECK(!in_destructor_);
-  in_destructor_ = true;
-  ClearConnections();
   app_lifetime_helper_.OnQuit();
 }
 
-ApplicationConnection* ApplicationImpl::ConnectToApplication(
+scoped_ptr<ApplicationConnection> ApplicationImpl::ConnectToApplication(
     mojo::URLRequestPtr request,
     CapabilityFilterPtr filter) {
   if (!shell_)
@@ -79,32 +75,12 @@ ApplicationConnection* ApplicationImpl::ConnectToApplication(
   //             filter here too?
   std::set<std::string> allowed;
   allowed.insert("*");
-  internal::ServiceRegistry* registry = new internal::ServiceRegistry(
-      this, application_url, application_url, remote_services.Pass(),
-      local_request.Pass(), allowed);
-  if (!delegate_->ConfigureOutgoingConnection(registry)) {
-    registry->CloseConnection();
+  scoped_ptr<ApplicationConnection> registry(new internal::ServiceRegistry(
+      application_url, application_url, remote_services.Pass(),
+      local_request.Pass(), allowed));
+  if (!delegate_->ConfigureOutgoingConnection(registry.get()))
     return nullptr;
-  }
-  outgoing_service_registries_.push_back(registry);
-  return registry;
-}
-
-void ApplicationImpl::CloseConnection(ApplicationConnection* connection) {
-  if (!in_destructor_)
-    delegate_->OnWillCloseConnection(connection);
-  auto outgoing_it = std::find(outgoing_service_registries_.begin(),
-                               outgoing_service_registries_.end(),
-                               connection);
-  if (outgoing_it != outgoing_service_registries_.end()) {
-    outgoing_service_registries_.erase(outgoing_it);
-    return;
-  }
-  auto incoming_it = std::find(incoming_service_registries_.begin(),
-                               incoming_service_registries_.end(),
-                               connection);
- if (incoming_it != incoming_service_registries_.end())
-    incoming_service_registries_.erase(incoming_it);
+  return registry.Pass();
 }
 
 void ApplicationImpl::Initialize(ShellPtr shell, const mojo::String& url) {
@@ -143,19 +119,18 @@ void ApplicationImpl::AcceptConnection(
     ServiceProviderPtr exposed_services,
     Array<String> allowed_interfaces,
     const String& url) {
-  internal::ServiceRegistry* registry = new internal::ServiceRegistry(
-      this, url, requestor_url, exposed_services.Pass(), services.Pass(),
-      allowed_interfaces.To<std::set<std::string>>());
-  if (!delegate_->ConfigureIncomingConnection(registry)) {
-    registry->CloseConnection();
+  scoped_ptr<ApplicationConnection> registry(new internal::ServiceRegistry(
+      url, requestor_url, exposed_services.Pass(), services.Pass(),
+      allowed_interfaces.To<std::set<std::string>>()));
+  if (!delegate_->ConfigureIncomingConnection(registry.get()))
     return;
-  }
-  incoming_service_registries_.push_back(registry);
 
   // If we were quitting because we thought there were no more services for this
   // app in use, then that has changed so cancel the quit request.
   if (quit_requested_)
     quit_requested_ = false;
+
+  incoming_connections_.push_back(registry.Pass());
 }
 
 void ApplicationImpl::OnQuitRequested(const Callback<void(bool)>& callback) {
@@ -182,20 +157,6 @@ void ApplicationImpl::OnConnectionError() {
   shell_ = nullptr;
 }
 
-void ApplicationImpl::ClearConnections() {
-  // Copy the ServiceRegistryLists because they will be mutated by
-  // ApplicationConnection::CloseConnection.
-  ServiceRegistryList incoming_service_registries(incoming_service_registries_);
-  for (internal::ServiceRegistry* registry : incoming_service_registries)
-    registry->CloseConnection();
-  DCHECK(incoming_service_registries_.empty());
-
-  ServiceRegistryList outgoing_service_registries(outgoing_service_registries_);
-  for (internal::ServiceRegistry* registry : outgoing_service_registries)
-    registry->CloseConnection();
-  DCHECK(outgoing_service_registries_.empty());
-}
-
 void ApplicationImpl::QuitNow() {
   delegate_->Quit();
   termination_closure_.Run();
diff --git a/mojo/application/public/cpp/lib/service_registry.cc b/mojo/application/public/cpp/lib/service_registry.cc
index 4f8e6e6..279209d 100644
--- a/mojo/application/public/cpp/lib/service_registry.cc
+++ b/mojo/application/public/cpp/lib/service_registry.cc
@@ -4,36 +4,38 @@
 
 #include "mojo/application/public/cpp/lib/service_registry.h"
 
+#include "base/logging.h"
 #include "mojo/application/public/cpp/application_connection.h"
-#include "mojo/application/public/cpp/application_impl.h"
 #include "mojo/application/public/cpp/service_connector.h"
 
 namespace mojo {
 namespace internal {
 
 ServiceRegistry::ServiceRegistry(
-    ApplicationImpl* application_impl,
     const std::string& connection_url,
     const std::string& remote_url,
     ServiceProviderPtr remote_services,
     InterfaceRequest<ServiceProvider> local_services,
     const std::set<std::string>& allowed_interfaces)
-    : application_impl_(application_impl),
-      connection_url_(connection_url),
+    : connection_url_(connection_url),
       remote_url_(remote_url),
       local_binding_(this),
       remote_service_provider_(remote_services.Pass()),
       allowed_interfaces_(allowed_interfaces),
       allow_all_interfaces_(allowed_interfaces_.size() == 1 &&
-                            allowed_interfaces_.count("*") == 1) {
+                            allowed_interfaces_.count("*") == 1),
+      weak_factory_(this) {
   if (local_services.is_pending())
     local_binding_.Bind(local_services.Pass());
 }
 
 ServiceRegistry::ServiceRegistry()
-    : application_impl_(nullptr),
-      local_binding_(this),
-      allow_all_interfaces_(true) {
+    : local_binding_(this),
+      allow_all_interfaces_(true),
+      weak_factory_(this) {
+}
+
+ServiceRegistry::~ServiceRegistry() {
 }
 
 void ServiceRegistry::SetServiceConnector(ServiceConnector* connector) {
@@ -63,6 +65,10 @@ void ServiceRegistry::SetRemoteServiceProviderConnectionErrorHandler(
   remote_service_provider_.set_connection_error_handler(handler);
 }
 
+base::WeakPtr<ApplicationConnection> ServiceRegistry::GetWeakPtr() {
+  return weak_factory_.GetWeakPtr();
+}
+
 void ServiceRegistry::RemoveServiceConnectorForName(
     const std::string& interface_name) {
   service_connector_registry_.RemoveServiceConnectorForName(interface_name);
@@ -82,14 +88,6 @@ ServiceProvider* ServiceRegistry::GetServiceProvider() {
   return remote_service_provider_.get();
 }
 
-ServiceRegistry::~ServiceRegistry() {
-}
-
-void ServiceRegistry::OnCloseConnection() {
-  if (application_impl_)
-    application_impl_->CloseConnection(this);
-}
-
 void ServiceRegistry::ConnectToService(const mojo::String& service_name,
                                        ScopedMessagePipeHandle client_handle) {
   service_connector_registry_.ConnectToService(this, service_name,
diff --git a/mojo/application/public/cpp/lib/service_registry.h b/mojo/application/public/cpp/lib/service_registry.h
index c34eced..7e4d604 100644
--- a/mojo/application/public/cpp/lib/service_registry.h
+++ b/mojo/application/public/cpp/lib/service_registry.h
@@ -13,10 +13,6 @@
 #include "mojo/application/public/interfaces/service_provider.mojom.h"
 
 namespace mojo {
-
-class Application;
-class ApplicationImpl;
-
 namespace internal {
 
 // A ServiceRegistry represents each half of a connection between two
@@ -28,12 +24,12 @@ class ServiceRegistry : public ServiceProvider, public ApplicationConnection {
   // |allowed_interfaces| are the set of interfaces that the shell has allowed
   // an application to expose to another application. If this set contains only
   // the string value "*" all interfaces may be exposed.
-  ServiceRegistry(ApplicationImpl* application_impl,
-                  const std::string& connection_url,
+  ServiceRegistry(const std::string& connection_url,
                   const std::string& remote_url,
                   ServiceProviderPtr remote_services,
                   InterfaceRequest<ServiceProvider> local_services,
                   const std::set<std::string>& allowed_interfaces);
+  ~ServiceRegistry() override;
 
   // ApplicationConnection overrides.
   void SetServiceConnector(ServiceConnector* service_connector) override;
@@ -45,20 +41,15 @@ class ServiceRegistry : public ServiceProvider, public ApplicationConnection {
   ServiceProvider* GetLocalServiceProvider() override;
   void SetRemoteServiceProviderConnectionErrorHandler(
       const Closure& handler) override;
+  base::WeakPtr<ApplicationConnection> GetWeakPtr() override;
 
   void RemoveServiceConnectorForName(const std::string& interface_name);
 
  private:
-  ~ServiceRegistry() override;
-
-  // ApplicationConnection overrides.
-  void OnCloseConnection() override;
-
   // ServiceProvider method.
   void ConnectToService(const mojo::String& service_name,
                         ScopedMessagePipeHandle client_handle) override;
 
-  ApplicationImpl* application_impl_;
   const std::string connection_url_;
   const std::string remote_url_;
   Binding<ServiceProvider> local_binding_;
@@ -66,6 +57,7 @@ class ServiceRegistry : public ServiceProvider, public ApplicationConnection {
   ServiceConnectorRegistry service_connector_registry_;
   const std::set<std::string> allowed_interfaces_;
   const bool allow_all_interfaces_;
+  base::WeakPtrFactory<ApplicationConnection> weak_factory_;
 
   MOJO_DISALLOW_COPY_AND_ASSIGN(ServiceRegistry);
 };
diff --git a/mojo/application/public/cpp/tests/BUILD.gn b/mojo/application/public/cpp/tests/BUILD.gn
index 561fc2a..29b35d8 100644
--- a/mojo/application/public/cpp/tests/BUILD.gn
+++ b/mojo/application/public/cpp/tests/BUILD.gn
@@ -10,6 +10,7 @@ test("mojo_public_application_unittests") {
   ]
 
   deps = [
+    "//base",
     "//mojo/application/public/cpp",
     "//testing/gtest",
     "//third_party/mojo/src/mojo/edk/test:run_all_unittests",
diff --git a/mojo/application/public/cpp/tests/service_registry_unittest.cc b/mojo/application/public/cpp/tests/service_registry_unittest.cc
index 4a9f28a..98ce637 100644
--- a/mojo/application/public/cpp/tests/service_registry_unittest.cc
+++ b/mojo/application/public/cpp/tests/service_registry_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "mojo/application/public/cpp/lib/service_registry.h"
 
+#include "base/memory/scoped_ptr.h"
 #include "mojo/application/public/cpp/service_connector.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -28,43 +29,40 @@ TEST(ServiceRegistryTest, Ownership) {
 
   // Destruction.
   {
-    ServiceRegistry* registry = new ServiceRegistry;
-    registry->SetServiceConnectorForName(new TestConnector(&delete_count),
-                                         "TC1");
-    registry->CloseConnection();
+    ServiceRegistry registry;
+    registry.SetServiceConnectorForName(new TestConnector(&delete_count),
+                                        "TC1");
   }
   EXPECT_EQ(1, delete_count);
 
   // Removal.
   {
-    ServiceRegistry* registry = new ServiceRegistry;
+    scoped_ptr<ServiceRegistry> registry(new ServiceRegistry);
     ServiceConnector* c = new TestConnector(&delete_count);
     registry->SetServiceConnectorForName(c, "TC1");
     registry->RemoveServiceConnectorForName("TC1");
-    registry->CloseConnection();
+    registry.reset();
     EXPECT_EQ(2, delete_count);
   }
 
   // Multiple.
   {
-    ServiceRegistry* registry = new ServiceRegistry;
-    registry->SetServiceConnectorForName(new TestConnector(&delete_count),
-                                         "TC1");
-    registry->SetServiceConnectorForName(new TestConnector(&delete_count),
-                                         "TC2");
-    registry->CloseConnection();
+    ServiceRegistry registry;
+    registry.SetServiceConnectorForName(new TestConnector(&delete_count),
+                                        "TC1");
+    registry.SetServiceConnectorForName(new TestConnector(&delete_count),
+                                        "TC2");
   }
   EXPECT_EQ(4, delete_count);
 
   // Re-addition.
   {
-    ServiceRegistry* registry = new ServiceRegistry;
-    registry->SetServiceConnectorForName(new TestConnector(&delete_count),
-                                         "TC1");
-    registry->SetServiceConnectorForName(new TestConnector(&delete_count),
-                                         "TC1");
+    ServiceRegistry registry;
+    registry.SetServiceConnectorForName(new TestConnector(&delete_count),
+                                        "TC1");
+    registry.SetServiceConnectorForName(new TestConnector(&delete_count),
+                                        "TC1");
     EXPECT_EQ(5, delete_count);
-    registry->CloseConnection();
   }
   EXPECT_EQ(6, delete_count);
 }
diff --git a/mojo/common/tracing_impl.cc b/mojo/common/tracing_impl.cc
index a8d2f36..46329300 100644
--- a/mojo/common/tracing_impl.cc
+++ b/mojo/common/tracing_impl.cc
@@ -20,8 +20,8 @@ TracingImpl::~TracingImpl() {
 void TracingImpl::Initialize(ApplicationImpl* app) {
   mojo::URLRequestPtr request(mojo::URLRequest::New());
   request->url = mojo::String::From("mojo:tracing");
-  ApplicationConnection* connection = app->ConnectToApplication(request.Pass());
-  connection->AddService(this);
+  connection_ = app->ConnectToApplication(request.Pass());
+  connection_->AddService(this);
 }
 
 void TracingImpl::Create(ApplicationConnection* connection,
diff --git a/mojo/common/tracing_impl.h b/mojo/common/tracing_impl.h
index d159b01..6299147 100644
--- a/mojo/common/tracing_impl.h
+++ b/mojo/common/tracing_impl.h
@@ -6,6 +6,7 @@
 #define MOJO_COMMON_TRACING_IMPL_H_
 
 #include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
 #include "mojo/application/public/cpp/interface_factory.h"
 #include "mojo/services/tracing/tracing.mojom.h"
 
@@ -27,6 +28,8 @@ class TracingImpl : public InterfaceFactory<tracing::TraceController> {
   void Create(ApplicationConnection* connection,
               InterfaceRequest<tracing::TraceController> request) override;
 
+  scoped_ptr<ApplicationConnection> connection_;
+
   DISALLOW_COPY_AND_ASSIGN(TracingImpl);
 };
 
diff --git a/mojo/mojo_base.gyp b/mojo/mojo_base.gyp
index 6b1d251..3187a98 100644
--- a/mojo/mojo_base.gyp
+++ b/mojo/mojo_base.gyp
@@ -244,7 +244,6 @@
         'application/public/cpp/interface_factory.h',
         'application/public/cpp/interface_factory_impl.h',
         'application/public/cpp/lib/app_lifetime_helper.cc',
-        'application/public/cpp/lib/application_connection.cc',
         'application/public/cpp/lib/application_delegate.cc',
         'application/public/cpp/lib/application_impl.cc',
         'application/public/cpp/lib/application_runner.cc',
diff --git a/mojo/services/network/http_server_apptest.cc b/mojo/services/network/http_server_apptest.cc
index 18c7e5c..f91b7e4 100644
--- a/mojo/services/network/http_server_apptest.cc
+++ b/mojo/services/network/http_server_apptest.cc
@@ -553,7 +553,7 @@ class HttpServerAppTest : public test::ApplicationTestBase {
 
     mojo::URLRequestPtr request(mojo::URLRequest::New());
     request->url = mojo::String::From("mojo:network_service");
-    ApplicationConnection* connection =
+    scoped_ptr<ApplicationConnection> connection =
         application_impl()->ConnectToApplication(request.Pass());
     connection->ConnectToService(&network_service_);
   }
diff --git a/mojo/services/network/udp_socket_apptest.cc b/mojo/services/network/udp_socket_apptest.cc
index eee66af..0c92ede 100644
--- a/mojo/services/network/udp_socket_apptest.cc
+++ b/mojo/services/network/udp_socket_apptest.cc
@@ -324,7 +324,7 @@ class UDPSocketAppTest : public test::ApplicationTestBase {
 
     mojo::URLRequestPtr request(mojo::URLRequest::New());
     request->url = mojo::String::From("mojo:network_service");
-    ApplicationConnection* connection =
+    scoped_ptr<ApplicationConnection> connection =
         application_impl()->ConnectToApplication(request.Pass());
     connection->ConnectToService(&network_service_);
 
diff --git a/mojo/shell/application_manager_unittest.cc b/mojo/shell/application_manager_unittest.cc
index 01c285c..e4bfb37 100644
--- a/mojo/shell/application_manager_unittest.cc
+++ b/mojo/shell/application_manager_unittest.cc
@@ -274,7 +274,8 @@ class TestAImpl : public TestA {
       : test_context_(test_context), binding_(this, request.Pass()) {
     mojo::URLRequestPtr request2(mojo::URLRequest::New());
     request2->url = mojo::String::From(kTestBURLString);
-    app_impl->ConnectToApplication(request2.Pass())->ConnectToService(&b_);
+    connection_ = app_impl->ConnectToApplication(request2.Pass());
+    connection_->ConnectToService(&b_);
   }
 
   ~TestAImpl() override {
@@ -298,6 +299,7 @@ class TestAImpl : public TestA {
     test_context_->QuitSoon();
   }
 
+  scoped_ptr<ApplicationConnection> connection_;
   TesterContext* test_context_;
   TestBPtr b_;
   StrongBinding<TestA> binding_;
diff --git a/mojo/shell/capability_filter_unittest.cc b/mojo/shell/capability_filter_unittest.cc
index 688815f..fda2c31 100644
--- a/mojo/shell/capability_filter_unittest.cc
+++ b/mojo/shell/capability_filter_unittest.cc
@@ -115,8 +115,8 @@ class ConnectionValidator : public ApplicationLoader,
 // set of interfaces exposed by that service.
 class TestApplication : public ApplicationDelegate {
  public:
-   TestApplication() : app_(nullptr) {}
-   ~TestApplication() override {}
+  TestApplication() : app_(nullptr) {}
+  ~TestApplication() override {}
 
  private:
   // Overridden from ApplicationDelegate:
@@ -129,17 +129,15 @@ class TestApplication : public ApplicationDelegate {
 
     URLRequestPtr request(URLRequest::New());
     request->url = String::From("test:service");
-    ApplicationConnection* connection1 =
-        app_->ConnectToApplication(request.Pass());
-    connection1->SetRemoteServiceProviderConnectionErrorHandler(
+    connection1_ = app_->ConnectToApplication(request.Pass());
+    connection1_->SetRemoteServiceProviderConnectionErrorHandler(
         base::Bind(&TestApplication::ConnectionClosed,
                    base::Unretained(this), "test:service"));
 
     URLRequestPtr request2(URLRequest::New());
     request2->url = String::From("test:service2");
-    ApplicationConnection* connection2 =
-        app_->ConnectToApplication(request2.Pass());
-    connection2->SetRemoteServiceProviderConnectionErrorHandler(
+    connection2_ = app_->ConnectToApplication(request2.Pass());
+    connection2_->SetRemoteServiceProviderConnectionErrorHandler(
         base::Bind(&TestApplication::ConnectionClosed,
                     base::Unretained(this), "test:service2"));
     return true;
@@ -151,6 +149,8 @@ class TestApplication : public ApplicationDelegate {
 
   ApplicationImpl* app_;
   ValidatorPtr validator_;
+  scoped_ptr<ApplicationConnection> connection1_;
+  scoped_ptr<ApplicationConnection> connection2_;
 
   DISALLOW_COPY_AND_ASSIGN(TestApplication);
 };
@@ -181,12 +181,15 @@ class TestContentHandler : public ApplicationDelegate,
   // Overridden from ContentHandler:
   void StartApplication(InterfaceRequest<Application> application,
                         URLResponsePtr response) override {
+    scoped_ptr<ApplicationDelegate> delegate(new TestApplication);
     embedded_apps_.push_back(
-        new ApplicationImpl(new TestApplication, application.Pass()));
+        new ApplicationImpl(delegate.get(), application.Pass()));
+    embedded_app_delegates_.push_back(delegate.Pass());
   }
 
   ApplicationImpl* app_;
   WeakBindingSet<ContentHandler> bindings_;
+  ScopedVector<ApplicationDelegate> embedded_app_delegates_;
   ScopedVector<ApplicationImpl> embedded_apps_;
 
   DISALLOW_COPY_AND_ASSIGN(TestContentHandler);