Make AppLifetimeHelper a member of ApplicationImpl.

This simplifies plumbing, but more so is so that ApplicationImpl can tell it when the application is gone. Otherwise AppLifetimeHelper might access a deleted ApplicationImpl after it's deleted when the app's message loop is being destructed (since an OnConnectionError might cause the last object holding a reference to end up calling ApplicationImpl::Terminate).

BUG=484234

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

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

6 files changed, 27 insertions, 9 deletions

diff --git a/mojo/application/public/cpp/app_lifetime_helper.h b/mojo/application/public/cpp/app_lifetime_helper.h
index dd5cb81..93005ef 100644
--- a/mojo/application/public/cpp/app_lifetime_helper.h
+++ b/mojo/application/public/cpp/app_lifetime_helper.h
@@ -11,6 +11,7 @@
 
 namespace mojo {
 
+class ApplicationImpl;
 class AppLifetimeHelper;
 
 // A service implementation should keep this object as a member variable to hold
@@ -56,7 +57,7 @@ class AppRefCount {
 // quit with a call to mojo::ApplicationImpl::Terminate().
 class AppLifetimeHelper {
  public:
-  AppLifetimeHelper();
+  explicit AppLifetimeHelper(ApplicationImpl* app);
   ~AppLifetimeHelper();
 
   scoped_ptr<AppRefCount> CreateAppRefCount();
@@ -66,6 +67,10 @@ class AppLifetimeHelper {
   void AddRef();
   void Release();
 
+  friend ApplicationImpl;
+  void ApplicationTerminated();
+
+  ApplicationImpl* app_;
   int ref_count_;
 
   DISALLOW_COPY_AND_ASSIGN(AppLifetimeHelper);
diff --git a/mojo/application/public/cpp/application_impl.h b/mojo/application/public/cpp/application_impl.h
index 8e04804..881421e 100644
--- a/mojo/application/public/cpp/application_impl.h
+++ b/mojo/application/public/cpp/application_impl.h
@@ -8,6 +8,7 @@
 #include <vector>
 
 #include "base/callback.h"
+#include "mojo/application/public/cpp/app_lifetime_helper.h"
 #include "mojo/application/public/cpp/application_connection.h"
 #include "mojo/application/public/cpp/application_delegate.h"
 #include "mojo/application/public/cpp/lib/service_registry.h"
@@ -72,6 +73,8 @@ class ApplicationImpl : public Application,
 
   const std::string& url() const { return url_; }
 
+  AppLifetimeHelper* app_lifetime_helper() { return &app_lifetime_helper_; }
+
   // 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
@@ -124,6 +127,7 @@ class ApplicationImpl : public Application,
   ShellPtr shell_;
   std::string url_;
   base::Closure termination_closure_;
+  AppLifetimeHelper app_lifetime_helper_;
 
   MOJO_DISALLOW_COPY_AND_ASSIGN(ApplicationImpl);
 };
diff --git a/mojo/application/public/cpp/lib/app_lifetime_helper.cc b/mojo/application/public/cpp/lib/app_lifetime_helper.cc
index 1360818..97a7f0f 100644
--- a/mojo/application/public/cpp/lib/app_lifetime_helper.cc
+++ b/mojo/application/public/cpp/lib/app_lifetime_helper.cc
@@ -63,8 +63,8 @@ scoped_ptr<AppRefCount> AppRefCount::Clone() {
       app_lifetime_helper_, app_task_runner_));
 }
 
-AppLifetimeHelper::AppLifetimeHelper()
-    : ref_count_(0) {
+AppLifetimeHelper::AppLifetimeHelper(ApplicationImpl* app)
+    : app_(app), ref_count_(0) {
 }
 
 AppLifetimeHelper::~AppLifetimeHelper() {
@@ -82,9 +82,15 @@ void AppLifetimeHelper::AddRef() {
 
 void AppLifetimeHelper::Release() {
   if (!--ref_count_) {
-    // Disabled until network_service tests pass again http://crbug.com/484234
-    //ApplicationImpl::Terminate();
+    if (app_) {
+      // Disabled until network_service tests pass again http://crbug.com/484234
+      //ApplicationImpl::Terminate();
+    }
   }
 }
 
+void AppLifetimeHelper::ApplicationTerminated() {
+  app_ = nullptr;
+}
+
 }  // namespace mojo
diff --git a/mojo/application/public/cpp/lib/application_impl.cc b/mojo/application/public/cpp/lib/application_impl.cc
index 564c773..8473a63 100644
--- a/mojo/application/public/cpp/lib/application_impl.cc
+++ b/mojo/application/public/cpp/lib/application_impl.cc
@@ -33,7 +33,8 @@ ApplicationImpl::ApplicationImpl(ApplicationDelegate* delegate,
                                  const base::Closure& termination_closure)
     : delegate_(delegate),
       binding_(this, request.Pass()),
-      termination_closure_(termination_closure) {
+      termination_closure_(termination_closure),
+      app_lifetime_helper_(this) {
 }
 
 void ApplicationImpl::ClearConnections() {
@@ -51,6 +52,7 @@ void ApplicationImpl::ClearConnections() {
 
 ApplicationImpl::~ApplicationImpl() {
   ClearConnections();
+  app_lifetime_helper_.ApplicationTerminated();
 }
 
 ApplicationConnection* ApplicationImpl::ConnectToApplication(
diff --git a/mojo/services/network/network_service_delegate.cc b/mojo/services/network/network_service_delegate.cc
index c6f4af5..70cf9e8 100644
--- a/mojo/services/network/network_service_delegate.cc
+++ b/mojo/services/network/network_service_delegate.cc
@@ -11,11 +11,12 @@
 #include "base/path_service.h"
 #include "mojo/application/public/cpp/application_connection.h"
 
-NetworkServiceDelegate::NetworkServiceDelegate() {}
+NetworkServiceDelegate::NetworkServiceDelegate() : app_(nullptr) {}
 
 NetworkServiceDelegate::~NetworkServiceDelegate() {}
 
 void NetworkServiceDelegate::Initialize(mojo::ApplicationImpl* app) {
+  app_ = app;
   base::FilePath base_path;
   CHECK(PathService::Get(base::DIR_TEMP, &base_path));
   base_path = base_path.Append(FILE_PATH_LITERAL("network_service"));
@@ -43,6 +44,6 @@ void NetworkServiceDelegate::Create(
       new mojo::NetworkServiceImpl(
           connection,
           context_.get(),
-          app_lifetime_helper_.CreateAppRefCount()),
+          app_->app_lifetime_helper()->CreateAppRefCount()),
       &request);
 }
diff --git a/mojo/services/network/network_service_delegate.h b/mojo/services/network/network_service_delegate.h
index 4deb421..2e98300 100644
--- a/mojo/services/network/network_service_delegate.h
+++ b/mojo/services/network/network_service_delegate.h
@@ -31,8 +31,8 @@ class NetworkServiceDelegate
               mojo::InterfaceRequest<mojo::NetworkService> request) override;
 
  private:
+  mojo::ApplicationImpl* app_;
   scoped_ptr<mojo::NetworkContext> context_;
-  mojo::AppLifetimeHelper app_lifetime_helper_;
 };
 
 #endif  // MOJO_SERVICES_NETWORK_NETWORK_SERVICE_DELEGATE_H_