Move us closer to shutting down cleanly.

This adds a Destroy() method to NativeViewportClient that the NVS can use to notify the app of NV destruction. The app should then halt its message loop and exit.

Note that we still crash on shutdown due to scheduled tasks continuing to want to touch the AcceleratedWidget post-destruction. We need to devise a better mechanism for this (see comment)

R=abarth@chromium.org
http://crbug.com/325901

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@239542 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 25 insertions, 10 deletions

diff --git a/mojo/examples/sample_app/native_viewport_client_impl.cc b/mojo/examples/sample_app/native_viewport_client_impl.cc
index 387a64e..2fc2d98 100644
--- a/mojo/examples/sample_app/native_viewport_client_impl.cc
+++ b/mojo/examples/sample_app/native_viewport_client_impl.cc
@@ -7,6 +7,7 @@
 #include <stdio.h>
 
 #include "base/logging.h"
+#include "base/message_loop/message_loop.h"
 
 namespace mojo {
 namespace examples {
@@ -31,10 +32,14 @@ void NativeViewportClientImpl::Open() {
   service_->CreateGLES2Context(gles2_client.Pass());
 }
 
-void NativeViewportClientImpl::DidOpen() {
+void NativeViewportClientImpl::OnCreated() {
 }
 
-void NativeViewportClientImpl::HandleEvent(const Event& event) {
+void NativeViewportClientImpl::OnDestroyed() {
+  base::MessageLoop::current()->Quit();
+}
+
+void NativeViewportClientImpl::OnEvent(const Event& event) {
   if (!event.location().is_null()) {
     LOG(INFO) << "Located Event @"
               << event.location().x() << "," << event.location().y();
diff --git a/mojo/examples/sample_app/native_viewport_client_impl.h b/mojo/examples/sample_app/native_viewport_client_impl.h
index c2b3de9..bd8ff5a 100644
--- a/mojo/examples/sample_app/native_viewport_client_impl.h
+++ b/mojo/examples/sample_app/native_viewport_client_impl.h
@@ -21,8 +21,9 @@ class NativeViewportClientImpl : public NativeViewportClientStub {
   void Open();
 
  private:
-  virtual void DidOpen() MOJO_OVERRIDE;
-  virtual void HandleEvent(const Event& event) MOJO_OVERRIDE;
+  virtual void OnCreated() MOJO_OVERRIDE;
+  virtual void OnDestroyed() MOJO_OVERRIDE;
+  virtual void OnEvent(const Event& event) MOJO_OVERRIDE;
 
   scoped_ptr<GLES2ClientImpl> gles2_client_;
 
diff --git a/mojo/services/native_viewport/native_viewport.mojom b/mojo/services/native_viewport/native_viewport.mojom
index 20348fe..aa7543b 100644
--- a/mojo/services/native_viewport/native_viewport.mojom
+++ b/mojo/services/native_viewport/native_viewport.mojom
@@ -29,8 +29,9 @@ interface NativeViewport {
 
 [Peer=NativeViewport]
 interface NativeViewportClient {
-  void DidOpen();
-  void HandleEvent(Event event);
+  void OnCreated();
+  void OnDestroyed();
+  void OnEvent(Event event);
 };
 
 }
diff --git a/mojo/services/native_viewport/native_viewport_impl.cc b/mojo/services/native_viewport/native_viewport_impl.cc
index 777c8d2..5b980c8 100644
--- a/mojo/services/native_viewport/native_viewport_impl.cc
+++ b/mojo/services/native_viewport/native_viewport_impl.cc
@@ -26,11 +26,10 @@ NativeViewportImpl::~NativeViewportImpl() {
 void NativeViewportImpl::Open() {
   native_viewport_ = services::NativeViewport::Create(context_, this);
   native_viewport_->Init();
-  client_->DidOpen();
+  client_->OnCreated();
 }
 
 void NativeViewportImpl::Close() {
-  gles2_.reset();
   DCHECK(native_viewport_);
   native_viewport_->Close();
 }
@@ -69,7 +68,7 @@ bool NativeViewportImpl::OnEvent(ui::Event* ui_event) {
     event.set_touch_data(touch_data.Finish());
   }
 
-  client_->HandleEvent(event.Finish());
+  client_->OnEvent(event.Finish());
   return false;
 }
 
@@ -83,7 +82,16 @@ void NativeViewportImpl::OnResized(const gfx::Size& size) {
 }
 
 void NativeViewportImpl::OnDestroyed() {
-  base::MessageLoop::current()->Quit();
+  // TODO(beng):
+  // Destroying |gles2_| on the shell thread here hits thread checker asserts.
+  // All code must stop touching the AcceleratedWidget at this point as it is
+  // dead after this call stack. jamesr said we probably should make our own
+  // GLSurface and simply tell it to stop touching the AcceleratedWidget
+  // via Destroy() but we have no good way of doing that right now given our
+  // current threading model so james' recommendation was just to wait until
+  // after we move the gl service out of process.
+  // gles2_.reset();
+  client_->OnDestroyed();
 }
 
 }  // namespace services