Mandoline: Introduce ApplicationConnection::CloseConnection

Close the ApplicationConnection when ViewManagerInit is destroyed (the browser is closed).

BUG=496935

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

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

8 files changed, 178 insertions, 35 deletions

diff --git a/mandoline/BUILD.gn b/mandoline/BUILD.gn
index 7a35fce..f835d35 100644
--- a/mandoline/BUILD.gn
+++ b/mandoline/BUILD.gn
@@ -37,6 +37,7 @@ group("tests") {
       "//components/view_manager:tests",
       "//sql/mojo:apptests",
       "//mandoline/tab:mandoline_frame_apptests",
+      "//mandoline/ui/browser:mandoline_browser_apptests",
       "//media/mojo/services:media_apptests",
     ]
   }
diff --git a/mandoline/ui/browser/BUILD.gn b/mandoline/ui/browser/BUILD.gn
index a8b6c1b..586eae8 100644
--- a/mandoline/ui/browser/BUILD.gn
+++ b/mandoline/ui/browser/BUILD.gn
@@ -29,6 +29,7 @@ source_set("lib") {
   sources = [
     "browser.cc",
     "browser.h",
+    "browser_delegate.h",
     "browser_manager.cc",
     "browser_manager.h",
     "navigator_host_impl.cc",
@@ -63,3 +64,19 @@ source_set("lib") {
     ]
   }
 }
+
+mojo_native_application("mandoline_browser_apptests") {
+  testonly = true
+
+  sources = [
+    "browser_apptest.cc",
+  ]
+
+  deps = [
+    "//base",
+    "//base/test:test_config",
+    "//mandoline/tab/public/interfaces",
+    "//mandoline/ui/browser:lib",
+    "//mojo/application/public/cpp:test_support",
+  ]
+}
diff --git a/mandoline/ui/browser/browser.cc b/mandoline/ui/browser/browser.cc
index 3cdd52d..f03d54b 100644
--- a/mandoline/ui/browser/browser.cc
+++ b/mandoline/ui/browser/browser.cc
@@ -11,7 +11,7 @@
 #include "mandoline/tab/frame.h"
 #include "mandoline/tab/frame_connection.h"
 #include "mandoline/tab/frame_tree.h"
-#include "mandoline/ui/browser/browser_manager.h"
+#include "mandoline/ui/browser/browser_delegate.h"
 #include "mandoline/ui/browser/browser_ui.h"
 #include "mojo/application/public/cpp/application_runner.h"
 #include "mojo/common/common_type_converters.h"
@@ -33,14 +33,14 @@ gfx::Size GetInitialViewportSize() {
 
 }  // namespace
 
-Browser::Browser(mojo::ApplicationImpl* app, BrowserManager* browser_manager)
+Browser::Browser(mojo::ApplicationImpl* app, BrowserDelegate* delegate)
     : view_manager_init_(app, this, this),
       root_(nullptr),
       content_(nullptr),
       omnibox_(nullptr),
       navigator_host_(this),
       app_(app),
-      browser_manager_(browser_manager) {
+      delegate_(delegate) {
   view_manager_init_.connection()->AddService<ViewEmbedder>(this);
 
   ui_.reset(BrowserUI::Create(this, app));
@@ -92,6 +92,10 @@ void Browser::OnDevicePixelRatioAvailable() {
   }
 }
 
+mojo::ApplicationConnection* Browser::GetViewManagerConnectionForTesting() {
+  return view_manager_init_.connection();
+}
+
 void Browser::OnEmbed(mojo::View* root) {
   // Browser does not support being embedded more than once.
   CHECK(!root_);
@@ -105,7 +109,7 @@ void Browser::OnEmbed(mojo::View* root) {
   //             the UI class.
   root_ = root;
 
-  if (!browser_manager_->InitUIIfNecessary(this, root_))
+  if (!delegate_->InitUIIfNecessary(this, root_))
     return;  // We'll be called back from OnDevicePixelRatioAvailable().
   OnDevicePixelRatioAvailable();
 }
@@ -135,7 +139,7 @@ void Browser::OnEmbedForDescendant(mojo::View* view,
 void Browser::OnViewManagerDestroyed(mojo::ViewManager* view_manager) {
   ui_.reset();
   root_ = nullptr;
-  browser_manager_->BrowserClosed(this);
+  delegate_->BrowserClosed(this);
 }
 
 void Browser::OnAccelerator(mojo::EventPtr event) {
diff --git a/mandoline/ui/browser/browser.h b/mandoline/ui/browser/browser.h
index e7d2c55..64954ba 100644
--- a/mandoline/ui/browser/browser.h
+++ b/mandoline/ui/browser/browser.h
@@ -5,6 +5,7 @@
 #ifndef MANDOLINE_UI_BROWSER_BROWSER_H_
 #define MANDOLINE_UI_BROWSER_BROWSER_H_
 
+#include "base/gtest_prod_util.h"
 #include "components/view_manager/public/cpp/view_manager.h"
 #include "components/view_manager/public/cpp/view_manager_delegate.h"
 #include "components/view_manager/public/cpp/view_manager_init.h"
@@ -26,7 +27,9 @@ class ViewManagerInit;
 
 namespace mandoline {
 
-class BrowserManager;
+FORWARD_DECLARE_TEST(BrowserTest, ClosingBrowserClosesAppConnection);
+
+class BrowserDelegate;
 class BrowserUI;
 class FrameTree;
 
@@ -37,7 +40,7 @@ class Browser : public mojo::ViewManagerDelegate,
                 public mojo::InterfaceFactory<mojo::NavigatorHost>,
                 public mojo::InterfaceFactory<ViewEmbedder> {
  public:
-  Browser(mojo::ApplicationImpl* app, BrowserManager* browser_manager);
+  Browser(mojo::ApplicationImpl* app, BrowserDelegate* delegate);
   ~Browser() override;
 
   void ReplaceContentWithRequest(mojo::URLRequestPtr request);
@@ -54,6 +57,10 @@ class Browser : public mojo::ViewManagerDelegate,
   void OnDevicePixelRatioAvailable();
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(BrowserTest, ClosingBrowserClosesAppConnection);
+
+  mojo::ApplicationConnection* GetViewManagerConnectionForTesting();
+
   // Overridden from mojo::ViewManagerDelegate:
   void OnEmbed(mojo::View* root) override;
   void OnEmbedForDescendant(mojo::View* view,
@@ -98,7 +105,7 @@ class Browser : public mojo::ViewManagerDelegate,
 
   scoped_ptr<BrowserUI> ui_;
   mojo::ApplicationImpl* app_;
-  BrowserManager* browser_manager_;
+  BrowserDelegate* delegate_;
 
   scoped_ptr<FrameTree> frame_tree_;
 
diff --git a/mandoline/ui/browser/browser_apptest.cc b/mandoline/ui/browser/browser_apptest.cc
new file mode 100644
index 0000000..fe0061b
--- /dev/null
+++ b/mandoline/ui/browser/browser_apptest.cc
@@ -0,0 +1,79 @@
+// 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 "mandoline/ui/browser/browser.h"
+
+#include "mandoline/ui/browser/browser_delegate.h"
+#include "mojo/application/public/cpp/application_connection.h"
+#include "mojo/application/public/cpp/application_delegate.h"
+#include "mojo/application/public/cpp/application_impl.h"
+#include "mojo/application/public/cpp/application_test_base.h"
+
+namespace mandoline {
+
+class BrowserTest : public mojo::test::ApplicationTestBase,
+                    public mojo::ApplicationDelegate,
+                    public BrowserDelegate {
+ public:
+  BrowserTest() : app_(nullptr), last_closed_connection_(nullptr) {}
+
+  // Creates a new Browser object.
+  Browser* CreateBrowser() {
+    if (!app_)
+      return nullptr;
+    Browser* browser = new Browser(app_, this);
+    browsers_.insert(browser);
+    return 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; }
+
+  // Overridden from BrowserDelegate:
+  void BrowserClosed(Browser* browser) override {
+    scoped_ptr<Browser> browser_owner(browser);
+    DCHECK_GT(browsers_.count(browser), 0u);
+    browsers_.erase(browser);
+  }
+
+  bool InitUIIfNecessary(Browser* browser, mojo::View* root_view) override {
+    return true;
+  }
+
+ private:
+  mojo::ApplicationImpl* app_;
+  void* last_closed_connection_;
+  std::set<Browser*> browsers_;
+
+  MOJO_DISALLOW_COPY_AND_ASSIGN(BrowserTest);
+};
+
+// This test verifies that closing a Browser closes the associated application
+// connection with the view manager.
+TEST_F(BrowserTest, ClosingBrowserClosesAppConnection) {
+  Browser* browser = CreateBrowser();
+  ASSERT_NE(nullptr, browser);
+  mojo::ApplicationConnection* view_manager_connection =
+      browser->view_manager_init_.connection();
+  ASSERT_NE(nullptr, view_manager_connection);
+  BrowserClosed(browser);
+  EXPECT_EQ(last_closed_connection(), view_manager_connection);
+}
+
+}  // namespace mandoline
diff --git a/mandoline/ui/browser/browser_delegate.h b/mandoline/ui/browser/browser_delegate.h
new file mode 100644
index 0000000..51985f6
--- /dev/null
+++ b/mandoline/ui/browser/browser_delegate.h
@@ -0,0 +1,34 @@
+// 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.
+
+#ifndef MANDOLINE_UI_BROWSER_BROWSER_DELEGATE_H_
+#define MANDOLINE_UI_BROWSER_BROWSER_DELEGATE_H_
+
+namespace mojo {
+class View;
+}
+
+namespace mandoline {
+
+class Browser;
+
+// A BrowserDelegate is an interface to be implemented by an object that manages
+// the lifetime of a Browser.
+class BrowserDelegate {
+ public:
+  // Invoken when the Browser wishes to close. This gives the delegate the
+  // opportunity to perform some cleanup.
+  virtual void BrowserClosed(Browser* browser) = 0;
+
+  // Requests initialization of state to display the browser on screen. Returns
+  // whether UI was initialized at this point.
+  virtual bool InitUIIfNecessary(Browser* browser, mojo::View* root_view) = 0;
+
+ protected:
+  virtual ~BrowserDelegate() {}
+};
+
+}  // mandoline
+
+#endif  // MANDOLINE_UI_BROWSER_BROWSER_DELEGATE_H_
diff --git a/mandoline/ui/browser/browser_manager.cc b/mandoline/ui/browser/browser_manager.cc
index 7d828fc..45e1180 100644
--- a/mandoline/ui/browser/browser_manager.cc
+++ b/mandoline/ui/browser/browser_manager.cc
@@ -65,28 +65,6 @@ Browser* BrowserManager::CreateBrowser() {
   return browser;
 }
 
-void BrowserManager::BrowserClosed(Browser* browser) {
-  scoped_ptr<Browser> browser_owner(browser);
-  DCHECK_GT(browsers_.count(browser), 0u);
-  browsers_.erase(browser);
-  if (browsers_.empty())
-    app_->Terminate();
-}
-
-bool BrowserManager::InitUIIfNecessary(Browser* browser, mojo::View* view) {
-  if (view->viewport_metrics().device_pixel_ratio > 0) {
-#if defined(USE_AURA)
-    if (!aura_init_)
-      aura_init_.reset(new AuraInit(view, app_->shell()));
-#endif
-    return true;
-  }
-  DCHECK(!device_pixel_ratio_waiter_.get());
-  device_pixel_ratio_waiter_.reset(
-      new DevicePixelRatioWaiter(this, browser, view));
-  return false;
-}
-
 void BrowserManager::OnDevicePixelRatioAvailable(Browser* browser,
                                                  mojo::View* view) {
   device_pixel_ratio_waiter_.reset();
@@ -117,6 +95,28 @@ bool BrowserManager::ConfigureIncomingConnection(
   return true;
 }
 
+void BrowserManager::BrowserClosed(Browser* browser) {
+  scoped_ptr<Browser> browser_owner(browser);
+  DCHECK_GT(browsers_.count(browser), 0u);
+  browsers_.erase(browser);
+  if (browsers_.empty())
+    app_->Terminate();
+}
+
+bool BrowserManager::InitUIIfNecessary(Browser* browser, mojo::View* view) {
+  if (view->viewport_metrics().device_pixel_ratio > 0) {
+#if defined(USE_AURA)
+    if (!aura_init_)
+      aura_init_.reset(new AuraInit(view, app_->shell()));
+#endif
+    return true;
+  }
+  DCHECK(!device_pixel_ratio_waiter_.get());
+  device_pixel_ratio_waiter_.reset(
+      new DevicePixelRatioWaiter(this, browser, view));
+  return false;
+}
+
 void BrowserManager::Create(mojo::ApplicationConnection* connection,
                             mojo::InterfaceRequest<LaunchHandler> request) {
   launch_handler_bindings_.AddBinding(this, request.Pass());
diff --git a/mandoline/ui/browser/browser_manager.h b/mandoline/ui/browser/browser_manager.h
index cb2927b..2624c81 100644
--- a/mandoline/ui/browser/browser_manager.h
+++ b/mandoline/ui/browser/browser_manager.h
@@ -8,6 +8,7 @@
 #include <set>
 
 #include "base/memory/scoped_vector.h"
+#include "mandoline/ui/browser/browser_delegate.h"
 #include "mandoline/ui/browser/public/interfaces/launch_handler.mojom.h"
 #include "mojo/application/public/cpp/application_delegate.h"
 #include "mojo/application/public/cpp/application_impl.h"
@@ -28,6 +29,7 @@ class Browser;
 
 // BrowserManager creates and manages the lifetime of Browsers.
 class BrowserManager : public mojo::ApplicationDelegate,
+                       public BrowserDelegate,
                        public LaunchHandler,
                        public mojo::InterfaceFactory<LaunchHandler> {
  public:
@@ -37,11 +39,6 @@ class BrowserManager : public mojo::ApplicationDelegate,
   // BrowserManager owns the returned Browser.
   Browser* CreateBrowser();
 
-  // Invoked by |browser| when it has closed.
-  void BrowserClosed(Browser* browser);
-
-  bool InitUIIfNecessary(Browser* browser, mojo::View* view);
-
  private:
   class DevicePixelRatioWaiter;
 
@@ -55,6 +52,10 @@ class BrowserManager : public mojo::ApplicationDelegate,
   bool ConfigureIncomingConnection(
       mojo::ApplicationConnection* connection) override;
 
+  // Overridden from BrowserDelegate:
+  bool InitUIIfNecessary(Browser* browser, mojo::View* view) override;
+  void BrowserClosed(Browser* browser) override;
+
   // Overridden from mojo::InterfaceFactory<LaunchHandler>:
   void Create(mojo::ApplicationConnection* connection,
               mojo::InterfaceRequest<LaunchHandler> request) override;