Make Mandoline shut down cleanly.

components/native_viewport/native_viewport_impl.cc

Index: components/native_viewport/native_viewport_impl.cc
diff --git a/components/native_viewport/native_viewport_impl.cc b/components/native_viewport/native_viewport_impl.cc
index 2f85500088553deddbbbaca9b657e83e1ddf9dfe..6233a48f86785a7f02f7fe1721c1b5cc6a5bb83f 100644
--- a/components/native_viewport/native_viewport_impl.cc
+++ b/components/native_viewport/native_viewport_impl.cc
@@ -20,9 +20,11 @@ namespace native_viewport {
 NativeViewportImpl::NativeViewportImpl(
     bool is_headless,
     const scoped_refptr<gles2::GpuState>& gpu_state,
-    mojo::InterfaceRequest<mojo::NativeViewport> request)
+    mojo::InterfaceRequest<mojo::NativeViewport> request,
+    scoped_ptr<mojo::AppRefCount> app_refcount)
     : is_headless_(is_headless),
-      context_provider_(gpu_state),
+      app_refcount_(app_refcount.Pass()),
+      context_provider_(new OnscreenContextProvider(gpu_state)),
       sent_metrics_(false),
       metrics_(mojo::ViewportMetrics::New()),
       binding_(this, request.Pass()),
@@ -31,6 +33,11 @@ NativeViewportImpl::NativeViewportImpl(
 }
 
 NativeViewportImpl::~NativeViewportImpl() {
+  // Destroy before |platform_viewport_| because this will destroy
+  // CommandBufferDriver objects that contain child windows. Otherwise if this
+  // class destroys its window first, X errors will occur.
+  context_provider_.reset();
+
   // Destroy the NativeViewport early on as it may call us back during
   // destruction and we want to be in a known state.
   platform_viewport_.reset();
@@ -76,7 +83,7 @@ void NativeViewportImpl::SetSize(mojo::SizePtr size) {
 
 void NativeViewportImpl::GetContextProvider(
     mojo::InterfaceRequest<mojo::ContextProvider> request) {
-  context_provider_.Bind(request.Pass());
+  context_provider_->Bind(request.Pass());
 }
 
 void NativeViewportImpl::SetEventDispatcher(
@@ -102,7 +109,7 @@ void NativeViewportImpl::OnAcceleratedWidgetAvailable(
     gfx::AcceleratedWidget widget,
     float device_pixel_ratio) {
   metrics_->device_pixel_ratio = device_pixel_ratio;
-  context_provider_.SetAcceleratedWidget(widget);
+  context_provider_->SetAcceleratedWidget(widget);
   // TODO: The metrics here might not match the actual window size on android
   // where we don't know the actual size until the first OnMetricsChanged call.
   create_callback_.Run(metrics_.Clone());
@@ -111,7 +118,7 @@ void NativeViewportImpl::OnAcceleratedWidgetAvailable(
 }
 
 void NativeViewportImpl::OnAcceleratedWidgetDestroyed() {
-  context_provider_.SetAcceleratedWidget(gfx::kNullAcceleratedWidget);
+  context_provider_->SetAcceleratedWidget(gfx::kNullAcceleratedWidget);
 }
 
 bool NativeViewportImpl::OnEvent(mojo::EventPtr event) {
@@ -150,8 +157,7 @@ bool NativeViewportImpl::OnEvent(mojo::EventPtr event) {
 }
 
 void NativeViewportImpl::OnDestroyed() {
-  // This will signal a connection error and cause us to delete |this|.
-  binding_.Close();
+  delete this;

[Ben Goodger (Google), 2015/05/14 22:57:18]

will this ever result in a double delete?

[jam, 2015/05/15 02:15:31]

Not that I saw.

[Ben Goodger (Google), 2015/05/15 16:19:07]

the reason why I ask is that see below in OnConnectionError. If the pipe is ever
broken this object will be deleted also. Perhaps that function should destroy
the platform viewport rather than deleting this object directly? Then this
function will be called by the platform viewport.

 }
 
 void NativeViewportImpl::OnConnectionError() {