Remove GLFW from the API surface (google#254)

Rather than exposing the GLFWwindow that is used, treat it as an
implementation detail and use an opaque pointer specific to this library
as the context object instead.

In practice the use of GLFW can't be entirely internal since the use of
GLFW in the library prevents its use by the embedder, but the details of
how it is used should not be exposed. This also gives greater control
over the API surface, as part of working toward a stable ABI (google#230).

This does mean that the embedder cannot manipulate the GLFW window
directly, but rather than trust that doing so would be safe across DLL
boundaries, any functionality need by the simple embedder
use cases this library enables should be exposed via wrappers.

Author: stuartmorgan

library/common/glfw/embedder.cc

@@ -20,6 +20,13 @@
 #include <cstdlib>
 #include <iostream>
 
+#ifdef __linux__
+// Epoxy must be included before any graphics-related code.
+#include <epoxy/gl.h>
+#endif
+
+#include <GLFW/glfw3.h>
+
 #include <flutter_embedder.h>
 
 #include "library/common/glfw/key_event_handler.h"
@@ -48,7 +55,13 @@ static constexpr double kDpPerInch = 160.0;
 
 // Struct for storing state within an instance of the GLFW Window.
 struct FlutterEmbedderState {
+  // The GLFW window that owns this state object.
+  GLFWwindow *window;
+
+  // The handle to the Flutter engine instance.
   FlutterEngine engine;
+
+  // The helper class managing plugin registration and messaging.
   std::unique_ptr<flutter_desktop_embedding::PluginHandler> plugin_handler;
 
   // Handlers for keyboard events from GLFW.
@@ -300,62 +313,69 @@ bool FlutterInit() {
 
 void FlutterTerminate() { glfwTerminate(); }
 
-PluginRegistrar *GetRegistrarForPlugin(GLFWwindow *flutter_window,
+PluginRegistrar *GetRegistrarForPlugin(FlutterWindowRef flutter_window,
                                        const std::string &plugin_name) {
-  auto *state = GetSavedEmbedderState(flutter_window);
   // Currently, PluginHandler acts as the registrar for all plugins, so the
   // name is ignored. It is part of the API to reduce churn in the future when
   // aligning more closely with the Flutter registrar system.
-  return state->plugin_handler.get();
+  return flutter_window->plugin_handler.get();
 }
 
-GLFWwindow *CreateFlutterWindow(size_t initial_width, size_t initial_height,
-                                const std::string &assets_path,
-                                const std::string &icu_data_path,
-                                const std::vector<std::string> &arguments) {
+FlutterWindowRef CreateFlutterWindow(
+    size_t initial_width, size_t initial_height, const std::string &assets_path,
+    const std::string &icu_data_path,
+    const std::vector<std::string> &arguments) {
 #ifdef __linux__
   gtk_init(0, nullptr);
 #endif
+  // Create the window.
   auto window = glfwCreateWindow(initial_width, initial_height,
                                  kDefaultWindowTitle, NULL, NULL);
   if (window == nullptr) {
     return nullptr;
   }
   GLFWClearCanvas(window);
+
+  // Start the engine.
   auto engine = RunFlutterEngine(window, assets_path, icu_data_path, arguments);
   if (engine == nullptr) {
     glfwDestroyWindow(window);
     return nullptr;
   }
 
+  // Create an embedder state object attached to the window.
   FlutterEmbedderState *state = new FlutterEmbedderState();
-  state->plugin_handler = std::make_unique<PluginHandler>(engine);
+  state->window = window;
+  glfwSetWindowUserPointer(window, state);
   state->engine = engine;
+  state->plugin_handler = std::make_unique<PluginHandler>(engine);
 
   // Set up the keyboard handlers.
   state->keyboard_hook_handlers.push_back(
       std::make_unique<KeyEventHandler>(state->plugin_handler.get()));
   state->keyboard_hook_handlers.push_back(
       std::make_unique<TextInputPlugin>(state->plugin_handler.get()));
 
-  glfwSetWindowUserPointer(window, state);
-
+  // Trigger an initial size callback to send size information to Flutter.
   state->monitor_screen_coordinates_per_inch = GetScreenCoordinatesPerInch();
   int width_px, height_px;
   glfwGetFramebufferSize(window, &width_px, &height_px);
-  glfwSetFramebufferSizeCallback(window, GLFWFramebufferSizeCallback);
   GLFWFramebufferSizeCallback(window, width_px, height_px);
 
+  // Set up GLFW callbacks for the window.
+  glfwSetFramebufferSizeCallback(window, GLFWFramebufferSizeCallback);
   GLFWAssignEventCallbacks(window);
-  return window;
+
+  return state;
 }
 
-void FlutterWindowLoop(GLFWwindow *flutter_window) {
+void FlutterWindowLoop(FlutterWindowRef flutter_window) {
+  GLFWwindow *window = flutter_window->window;
 #ifdef __linux__
   // Necessary for GTK thread safety.
   XInitThreads();
 #endif
-  while (!glfwWindowShouldClose(flutter_window)) {
+  while (!glfwWindowShouldClose(window)) {
 #ifdef __linux__
     glfwPollEvents();
     if (gtk_events_pending()) {
@@ -367,10 +387,9 @@ void FlutterWindowLoop(GLFWwindow *flutter_window) {
     // TODO(awdavies): This will be deprecated soon.
     __FlutterEngineFlushPendingTasksNow();
   }
-  auto state = GetSavedEmbedderState(flutter_window);
-  FlutterEngineShutdown(state->engine);
-  delete state;
-  glfwDestroyWindow(flutter_window);
+  FlutterEngineShutdown(flutter_window->engine);
+  delete flutter_window;
+  glfwDestroyWindow(window);
 }
 
 }  // namespace flutter_desktop_embedding

library/include/flutter_desktop_embedding/glfw/embedder.h

@@ -19,13 +19,6 @@
 #include <string>
 #include <vector>
 
-#ifdef __linux__
-// Epoxy must be included before any graphics-related code.
-#include <epoxy/gl.h>
-#endif
-
-#include <GLFW/glfw3.h>
-
 #ifdef USE_FLATTENED_INCLUDES
 #include "fde_export.h"
 #include "plugin_registrar.h"
@@ -34,19 +27,23 @@
 #include "../plugin_registrar.h"
 #endif
 
+// Opaque reference to a Flutter window.
+typedef struct FlutterEmbedderState *FlutterWindowRef;
+
 namespace flutter_desktop_embedding {
 
-// Calls glfwInit()
+// Sets up the embedder's graphic context. Must be called before any other
+// methods.
 //
-// glfwInit() must be called in the same library as glfwCreateWindow()
+// Note: Internally, this library uses GLFW, which does not support multiple
+// copies within the same process. Internally this calls glfwInit, which will
+// fail if you have called glfwInit elsewhere in the process.
 FDE_EXPORT bool FlutterInit();
 
-// Calls glfwTerminate()
-//
-// glfwTerminate() must be called in the same library as glfwCreateWindow()
+// Tears down embedder state. Must be called before the process terminates.
 FDE_EXPORT void FlutterTerminate();
 
-// Creates a GLFW Window running a Flutter Application.
+// Creates a Window running a Flutter Application.
 //
 // FlutterInit() must be called prior to this function.
 //
@@ -58,9 +55,11 @@ FDE_EXPORT void FlutterTerminate();
 // https://github.com/flutter/engine/blob/master/shell/common/switches.h for
 // for details. Not all arguments will apply to embedding mode.
 //
-// Returns a null pointer in the event of an error. The caller owns the pointer
-// when it is non-null.
-FDE_EXPORT GLFWwindow *CreateFlutterWindow(
+// Returns a null pointer in the event of an error. Otherwise, the pointer is
+// valid until FlutterWindowLoop has been called and returned. Note that calling
+// CreateFlutterWindow without later calling FlutterWindowLoop on that pointer
+// is a memory leak.
+FDE_EXPORT FlutterWindowRef CreateFlutterWindow(
     size_t initial_width, size_t initial_height, const std::string &assets_path,
     const std::string &icu_data_path,
     const std::vector<std::string> &arguments);
@@ -71,16 +70,13 @@ FDE_EXPORT GLFWwindow *CreateFlutterWindow(
 // The name must be unique across the application, so the recommended approach
 // is to use the fully namespace-qualified name of the plugin class.
 FDE_EXPORT PluginRegistrar *GetRegistrarForPlugin(
-    GLFWwindow *flutter_window, const std::string &plugin_name);
+    FlutterWindowRef flutter_window, const std::string &plugin_name);
 
-// Loops on flutter window events until termination.
-//
-// Must be used instead of glfwWindowShouldClose as it cleans up engine state
-// after termination.
+// Loops on Flutter window events until the window is closed.
 //
-// After this function the user must eventually call FlutterTerminate() if doing
-// cleanup.
-FDE_EXPORT void FlutterWindowLoop(GLFWwindow *flutter_window);
+// Once this function returns, FlutterWindowRef is no longer valid, and must
+// not be used again.
+FDE_EXPORT void FlutterWindowLoop(FlutterWindowRef flutter_window);
 
 }  // namespace flutter_desktop_embedding
 

library/include/flutter_desktop_embedding/glfw/flutter_window_controller.h

@@ -81,7 +81,7 @@ class FDE_EXPORT FlutterWindowController {
   bool init_succeeded_ = false;
 
   // The curent Flutter window, if any.
-  GLFWwindow *window_ = nullptr;
+  FlutterWindowRef window_ = nullptr;
 };
 
 }  // namespace flutter_desktop_embedding