Add RAII wrapper for EGLSurface

shell/platform/android/android_context_gl.cc

@@ -105,6 +105,14 @@ static bool TeardownContext(EGLDisplay display, EGLContext context) {
   return true;
 }
 
+AndroidEGLSurface::AndroidEGLSurface(EGLSurface surface, EGLDisplay display)
+    : surface(surface), display_(display) {}
+
+AndroidEGLSurface::~AndroidEGLSurface() {
+  auto result = eglDestroySurface(display_, surface);
+  FML_DCHECK(result == EGL_TRUE);
+}
+
 AndroidContextGL::AndroidContextGL(
     AndroidRenderingAPI rendering_api,
     fml::RefPtr<AndroidEnvironmentGL> environment)
@@ -161,25 +169,28 @@ AndroidContextGL::~AndroidContextGL() {
   }
 }
 
-EGLSurface AndroidContextGL::CreateOnscreenSurface(
+std::unique_ptr<AndroidEGLSurface> AndroidContextGL::CreateOnscreenSurface(
     fml::RefPtr<AndroidNativeWindow> window) const {
   EGLDisplay display = environment_->Display();
 
   const EGLint attribs[] = {EGL_NONE};
 
-  return eglCreateWindowSurface(
+  EGLSurface surface = eglCreateWindowSurface(
       display, config_, reinterpret_cast<EGLNativeWindowType>(window->handle()),
       attribs);
+  return std::make_unique<AndroidEGLSurface>(surface, display);
 }
 
-EGLSurface AndroidContextGL::CreateOffscreenSurface() const {
+std::unique_ptr<AndroidEGLSurface> AndroidContextGL::CreateOffscreenSurface()
+    const {
   // We only ever create pbuffer surfaces for background resource loading
   // contexts. We never bind the pbuffer to anything.
   EGLDisplay display = environment_->Display();
 
   const EGLint attribs[] = {EGL_WIDTH, 1, EGL_HEIGHT, 1, EGL_NONE};
 
-  return eglCreatePbufferSurface(display, config_, attribs);
+  EGLSurface surface = eglCreatePbufferSurface(display, config_, attribs);
+  return std::make_unique<AndroidEGLSurface>(surface, display);
 }
 
 fml::RefPtr<AndroidEnvironmentGL> AndroidContextGL::Environment() const {
@@ -190,19 +201,21 @@ bool AndroidContextGL::IsValid() const {
   return valid_;
 }
 
-bool AndroidContextGL::MakeCurrent(EGLSurface& surface) {
-  if (eglMakeCurrent(environment_->Display(), surface, surface, context_) !=
-      EGL_TRUE) {
+bool AndroidContextGL::MakeCurrent(
+    std::unique_ptr<AndroidEGLSurface> surface_wrapper) {
+  if (eglMakeCurrent(environment_->Display(), surface_wrapper->surface,
+                     surface_wrapper->surface, context_) != EGL_TRUE) {
     FML_LOG(ERROR) << "Could not make the context current";
     LogLastEGLError();
     return false;
   }
   return true;
 }
 
-bool AndroidContextGL::ResourceMakeCurrent(EGLSurface& surface) {
-  if (eglMakeCurrent(environment_->Display(), surface, surface,
-                     resource_context_) != EGL_TRUE) {
+bool AndroidContextGL::ResourceMakeCurrent(
+    std::unique_ptr<AndroidEGLSurface> surface_wrapper) {
+  if (eglMakeCurrent(environment_->Display(), surface_wrapper->surface,
+                     surface_wrapper->surface, resource_context_) != EGL_TRUE) {
     FML_LOG(ERROR) << "Could not make the context current";
     LogLastEGLError();
     return false;
@@ -223,30 +236,26 @@ bool AndroidContextGL::ClearCurrent() {
   return true;
 }
 
-bool AndroidContextGL::SwapBuffers(EGLSurface& surface) {
+bool AndroidContextGL::SwapBuffers(
+    std::unique_ptr<AndroidEGLSurface> surface_wrapper) {
   TRACE_EVENT0("flutter", "AndroidContextGL::SwapBuffers");
-  return eglSwapBuffers(environment_->Display(), surface);
+  return eglSwapBuffers(environment_->Display(), surface_wrapper->surface);
 }
 
-SkISize AndroidContextGL::GetSize(EGLSurface& surface) {
+SkISize AndroidContextGL::GetSize(
+    std::unique_ptr<AndroidEGLSurface> surface_wrapper) {
   EGLint width = 0;
   EGLint height = 0;
 
-  if (!eglQuerySurface(environment_->Display(), surface, EGL_WIDTH, &width) ||
-      !eglQuerySurface(environment_->Display(), surface, EGL_HEIGHT, &height)) {
+  if (!eglQuerySurface(environment_->Display(), surface_wrapper->surface,
+                       EGL_WIDTH, &width) ||
+      !eglQuerySurface(environment_->Display(), surface_wrapper->surface,
+                       EGL_HEIGHT, &height)) {
     FML_LOG(ERROR) << "Unable to query EGL surface size";
     LogLastEGLError();
     return SkISize::Make(0, 0);
   }
   return SkISize::Make(width, height);
 }
 
-bool AndroidContextGL::TeardownSurface(EGLSurface& surface) {
-  if (surface != EGL_NO_SURFACE) {
-    return eglDestroySurface(environment_->Display(), surface) == EGL_TRUE;
-  }
-
-  return true;
-}
-
 }  // namespace flutter

shell/platform/android/android_context_gl.h

@@ -16,6 +16,23 @@
 
 namespace flutter {
 
+//------------------------------------------------------------------------------
+/// Holds an `EGLSurface` reference.
+///
+///
+/// This can be used in conjuction to unique_ptr to provide better guarantees
+/// about the lifespam of the `EGLSurface` object.
+///
+struct AndroidEGLSurface {
+  AndroidEGLSurface(EGLSurface surface, EGLDisplay display);
+  ~AndroidEGLSurface();
+
+  const EGLSurface surface;
+
+ private:
+  const EGLDisplay display_;
+};
+
 //------------------------------------------------------------------------------
 /// The Android context is used by `AndroidSurfaceGL` to create and manage
 /// EGL surfaces.
@@ -34,24 +51,18 @@ class AndroidContextGL : public AndroidContext {
   /// @brief      Allocates an new EGL window surface that is used for on-screen
   ///             pixels.
   ///
-  /// @attention  Consumers must tear down the surface by calling
-  ///             `AndroidContextGL::TeardownSurface`.
-  ///
   /// @return     The window surface.
   ///
-  EGLSurface CreateOnscreenSurface(
+  std::unique_ptr<AndroidEGLSurface> CreateOnscreenSurface(
       fml::RefPtr<AndroidNativeWindow> window) const;
 
   //----------------------------------------------------------------------------
   /// @brief      Allocates an 1x1 pbuffer surface that is used for making the
   ///             offscreen current for texture uploads.
   ///
-  /// @attention  Consumers must tear down the surface by calling
-  ///             `AndroidContextGL::TeardownSurface`.
-  ///
   /// @return     The pbuffer surface.
   ///
-  EGLSurface CreateOffscreenSurface() const;
+  std::unique_ptr<AndroidEGLSurface> CreateOffscreenSurface() const;
 
   //----------------------------------------------------------------------------
   /// @return     The Android environment that contains a reference to the
@@ -77,35 +88,28 @@ class AndroidContextGL : public AndroidContext {
   ///
   /// @return     Whether the surface was made current.
   ///
-  bool MakeCurrent(EGLSurface& surface);
+  bool MakeCurrent(std::unique_ptr<AndroidEGLSurface> surface_wrapper);
 
   //----------------------------------------------------------------------------
   /// @brief      Binds the resource EGLContext context to the current rendering
   ///             thread and to the draw and read surface.
   ///
   /// @return     Whether the surface was made current.
   ///
-  bool ResourceMakeCurrent(EGLSurface& surface);
+  bool ResourceMakeCurrent(std::unique_ptr<AndroidEGLSurface> surface_wrapper);
 
   //----------------------------------------------------------------------------
   /// @brief      This only applies to on-screen surfaces such as those created
   ///             by `AndroidContextGL::CreateOnscreenSurface`.
   ///
   /// @return     Whether the EGL surface color buffer was swapped.
   ///
-  bool SwapBuffers(EGLSurface& surface);
+  bool SwapBuffers(std::unique_ptr<AndroidEGLSurface> surface_wrapper);
 
   //----------------------------------------------------------------------------
   /// @return     The size of an `EGLSurface`.
   ///
-  SkISize GetSize(EGLSurface& surface);
-
-  //----------------------------------------------------------------------------
-  /// @brief      Destroys an `EGLSurface`.
-  ///
-  /// @return     Whether the surface was destroyed.
-  ///
-  bool TeardownSurface(EGLSurface& surface);
+  SkISize GetSize(std::unique_ptr<AndroidEGLSurface> surface_wrapper);
 
  private:
   fml::RefPtr<AndroidEnvironmentGL> environment_;

shell/platform/android/android_surface_gl.cc

@@ -20,22 +20,13 @@ AndroidSurfaceGL::AndroidSurfaceGL(
       std::static_pointer_cast<AndroidContextGL>(android_context);
   // Acquire the offscreen surface.
   offscreen_surface_ = android_context_->CreateOffscreenSurface();
-  if (offscreen_surface_ == EGL_NO_SURFACE) {
+  if (offscreen_surface_->surface == EGL_NO_SURFACE) {
     offscreen_surface_ = nullptr;
   }
   external_view_embedder_ = std::make_unique<AndroidExternalViewEmbedder>();
 }
 
-AndroidSurfaceGL::~AndroidSurfaceGL() {
-  if (offscreen_surface_) {
-    android_context_->TeardownSurface(offscreen_surface_);
-    offscreen_surface_ = nullptr;
-  }
-  if (onscreen_surface_) {
-    android_context_->TeardownSurface(onscreen_surface_);
-    onscreen_surface_ = nullptr;
-  }
-}
+AndroidSurfaceGL::~AndroidSurfaceGL() = default;
 
 void AndroidSurfaceGL::TeardownOnScreenContext() {
   android_context_->ClearCurrent();
@@ -58,25 +49,24 @@ bool AndroidSurfaceGL::OnScreenSurfaceResize(const SkISize& size) {
   FML_DCHECK(onscreen_surface_);
   FML_DCHECK(native_window_);
 
-  if (size == android_context_->GetSize(onscreen_surface_)) {
+  if (size == android_context_->GetSize(std::move(onscreen_surface_))) {
     return true;
   }
 
   android_context_->ClearCurrent();
-  android_context_->TeardownSurface(onscreen_surface_);
 
   onscreen_surface_ = android_context_->CreateOnscreenSurface(native_window_);
-  if (!onscreen_surface_ || onscreen_surface_ == EGL_NO_SURFACE) {
+  if (onscreen_surface_->surface == EGL_NO_SURFACE) {
     FML_LOG(ERROR) << "Unable to create EGL window surface on resize.";
     return false;
   }
-  android_context_->MakeCurrent(onscreen_surface_);
+  android_context_->MakeCurrent(std::move(onscreen_surface_));
   return true;
 }
 
 bool AndroidSurfaceGL::ResourceContextMakeCurrent() {
   FML_DCHECK(IsValid());
-  return android_context_->ResourceMakeCurrent(offscreen_surface_);
+  return android_context_->ResourceMakeCurrent(std::move(offscreen_surface_));
 }
 
 bool AndroidSurfaceGL::ResourceContextClearCurrent() {
@@ -91,7 +81,7 @@ bool AndroidSurfaceGL::SetNativeWindow(
   native_window_ = window;
   // Create the onscreen surface.
   onscreen_surface_ = android_context_->CreateOnscreenSurface(window);
-  if (onscreen_surface_ == EGL_NO_SURFACE) {
+  if (onscreen_surface_->surface == EGL_NO_SURFACE) {
     return false;
   }
   return true;
@@ -101,7 +91,7 @@ std::unique_ptr<GLContextResult> AndroidSurfaceGL::GLContextMakeCurrent() {
   FML_DCHECK(IsValid());
   FML_DCHECK(onscreen_surface_);
   auto default_context_result = std::make_unique<GLContextDefaultResult>(
-      android_context_->MakeCurrent(onscreen_surface_));
+      android_context_->MakeCurrent(std::move(onscreen_surface_)));
   return std::move(default_context_result);
 }
 
@@ -113,7 +103,7 @@ bool AndroidSurfaceGL::GLContextClearCurrent() {
 bool AndroidSurfaceGL::GLContextPresent() {
   FML_DCHECK(IsValid());
   FML_DCHECK(onscreen_surface_);
-  return android_context_->SwapBuffers(onscreen_surface_);
+  return android_context_->SwapBuffers(std::move(onscreen_surface_));
 }
 
 intptr_t AndroidSurfaceGL::GLContextFBO() const {

shell/platform/android/android_surface_gl.h

@@ -64,8 +64,8 @@ class AndroidSurfaceGL final : public GPUSurfaceGLDelegate,
   fml::RefPtr<AndroidNativeWindow> native_window_;
   std::unique_ptr<AndroidExternalViewEmbedder> external_view_embedder_;
   std::shared_ptr<AndroidContextGL> android_context_;
-  EGLSurface onscreen_surface_;
-  EGLSurface offscreen_surface_;
+  std::unique_ptr<AndroidEGLSurface> onscreen_surface_;
+  std::unique_ptr<AndroidEGLSurface> offscreen_surface_;
 
   FML_DISALLOW_COPY_AND_ASSIGN(AndroidSurfaceGL);
 };