Reland: Replaces bespoke call captures from mock gles with gmock (flutter/engine#57019)

relands flutter/engine#56995

It previously landed with stale presubmits, this should be up to date now.  I have the fix in a separate commit.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

Author: gaaclarke

engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles_unittests.cc

@@ -12,12 +12,18 @@
 namespace impeller {
 namespace testing {
 
+using ::testing::_;
+
 TEST(BufferBindingsGLESTest, BindUniformData) {
   BufferBindingsGLES bindings;
   absl::flat_hash_map<std::string, GLint> uniform_bindings;
   uniform_bindings["SHADERMETADATA.FOOBAR"] = 1;
   bindings.SetUniformBindings(std::move(uniform_bindings));
-  std::shared_ptr<MockGLES> mock_gl = MockGLES::Init();
+  auto mock_gles_impl = std::make_unique<MockGLESImpl>();
+
+  EXPECT_CALL(*mock_gles_impl, Uniform1fv(_, _, _)).Times(1);
+
+  std::shared_ptr<MockGLES> mock_gl = MockGLES::Init(std::move(mock_gles_impl));
   std::vector<BufferResource> bound_buffers;
   std::vector<TextureAndSampler> bound_textures;
 
@@ -39,9 +45,6 @@ TEST(BufferBindingsGLESTest, BindUniformData) {
   EXPECT_TRUE(bindings.BindUniformData(mock_gl->GetProcTable(), bound_textures,
                                        bound_buffers, Range{0, 0},
                                        Range{0, 1}));
-  std::vector<std::string> captured_calls = mock_gl->GetCapturedCalls();
-  EXPECT_TRUE(std::find(captured_calls.begin(), captured_calls.end(),
-                        "glUniform1fv") != captured_calls.end());
 }
 
 }  // namespace testing

engine/src/flutter/impeller/renderer/backend/gles/test/capabilities_unittests.cc

@@ -33,40 +33,38 @@ TEST(CapabilitiesGLES, CanInitializeWithDefaults) {
 }
 
 TEST(CapabilitiesGLES, SupportsDecalSamplerAddressMode) {
-  auto const extensions = std::vector<const unsigned char*>{
-      reinterpret_cast<const unsigned char*>("GL_KHR_debug"),                 //
-      reinterpret_cast<const unsigned char*>("GL_EXT_texture_border_clamp"),  //
+  auto const extensions = std::vector<const char*>{
+      "GL_KHR_debug",                 //
+      "GL_EXT_texture_border_clamp",  //
   };
   auto mock_gles = MockGLES::Init(extensions);
   auto capabilities = mock_gles->GetProcTable().GetCapabilities();
   EXPECT_TRUE(capabilities->SupportsDecalSamplerAddressMode());
 }
 
 TEST(CapabilitiesGLES, SupportsDecalSamplerAddressModeNotOES) {
-  auto const extensions = std::vector<const unsigned char*>{
-      reinterpret_cast<const unsigned char*>("GL_KHR_debug"),                 //
-      reinterpret_cast<const unsigned char*>("GL_OES_texture_border_clamp"),  //
+  auto const extensions = std::vector<const char*>{
+      "GL_KHR_debug",                 //
+      "GL_OES_texture_border_clamp",  //
   };
   auto mock_gles = MockGLES::Init(extensions);
   auto capabilities = mock_gles->GetProcTable().GetCapabilities();
   EXPECT_FALSE(capabilities->SupportsDecalSamplerAddressMode());
 }
 
 TEST(CapabilitiesGLES, SupportsFramebufferFetch) {
-  auto const extensions = std::vector<const unsigned char*>{
-      reinterpret_cast<const unsigned char*>("GL_KHR_debug"),  //
-      reinterpret_cast<const unsigned char*>(
-          "GL_EXT_shader_framebuffer_fetch"),  //
+  auto const extensions = std::vector<const char*>{
+      "GL_KHR_debug",                     //
+      "GL_EXT_shader_framebuffer_fetch",  //
   };
   auto mock_gles = MockGLES::Init(extensions);
   auto capabilities = mock_gles->GetProcTable().GetCapabilities();
   EXPECT_TRUE(capabilities->SupportsFramebufferFetch());
 }
 
 TEST(CapabilitiesGLES, SupportsMSAA) {
-  auto const extensions = std::vector<const unsigned char*>{
-      reinterpret_cast<const unsigned char*>(
-          "GL_EXT_multisampled_render_to_texture"),
+  auto const extensions = std::vector<const char*>{
+      "GL_EXT_multisampled_render_to_texture",
   };
   auto mock_gles = MockGLES::Init(extensions);
   auto capabilities = mock_gles->GetProcTable().GetCapabilities();

engine/src/flutter/impeller/renderer/backend/gles/test/gpu_tracer_gles_unittests.cc

@@ -10,38 +10,48 @@
 namespace impeller {
 namespace testing {
 
+using ::testing::_;
+
 #ifdef IMPELLER_DEBUG
 TEST(GPUTracerGLES, CanFormatFramebufferErrorMessage) {
-  auto const extensions = std::vector<const unsigned char*>{
-      reinterpret_cast<const unsigned char*>("GL_KHR_debug"),                 //
-      reinterpret_cast<const unsigned char*>("GL_EXT_disjoint_timer_query"),  //
+  auto const extensions = std::vector<const char*>{
+      "GL_KHR_debug",                 //
+      "GL_EXT_disjoint_timer_query",  //
   };
-  auto mock_gles = MockGLES::Init(extensions);
+  auto mock_gles_impl = std::make_unique<MockGLESImpl>();
+
+  {
+    ::testing::InSequence seq;
+    auto gen_queries = [](GLsizei n, GLuint* ids) {
+      for (int i = 0; i < n; ++i) {
+        ids[i] = i + 1;
+      }
+    };
+    EXPECT_CALL(*mock_gles_impl, GenQueriesEXT(_, _)).WillOnce(gen_queries);
+    EXPECT_CALL(*mock_gles_impl, BeginQueryEXT(GL_TIME_ELAPSED_EXT, _));
+    EXPECT_CALL(*mock_gles_impl, EndQueryEXT(GL_TIME_ELAPSED_EXT));
+    EXPECT_CALL(*mock_gles_impl,
+                GetQueryObjectuivEXT(_, GL_QUERY_RESULT_AVAILABLE_EXT, _))
+        .WillOnce([](GLuint id, GLenum target, GLuint* result) {
+          *result = GL_TRUE;
+        });
+    EXPECT_CALL(*mock_gles_impl,
+                GetQueryObjectui64vEXT(_, GL_QUERY_RESULT_EXT, _))
+        .WillOnce([](GLuint id, GLenum target, GLuint64* result) {
+          *result = 1000u;
+        });
+    EXPECT_CALL(*mock_gles_impl, DeleteQueriesEXT(_, _));
+    EXPECT_CALL(*mock_gles_impl, GenQueriesEXT(_, _)).WillOnce(gen_queries);
+    EXPECT_CALL(*mock_gles_impl, BeginQueryEXT(GL_TIME_ELAPSED_EXT, _));
+  }
+  std::shared_ptr<MockGLES> mock_gles =
+      MockGLES::Init(std::move(mock_gles_impl), extensions);
   auto tracer =
       std::make_shared<GPUTracerGLES>(mock_gles->GetProcTable(), true);
   tracer->RecordRasterThread();
   tracer->MarkFrameStart(mock_gles->GetProcTable());
   tracer->MarkFrameEnd(mock_gles->GetProcTable());
-
-  auto calls = mock_gles->GetCapturedCalls();
-
-  std::vector<std::string> expected = {"glGenQueriesEXT", "glBeginQueryEXT",
-                                       "glEndQueryEXT"};
-  for (auto i = 0; i < 3; i++) {
-    EXPECT_EQ(calls[i], expected[i]);
-  }
-
-  // Begin second frame, which prompts the tracer to query the result
-  // from the previous frame.
   tracer->MarkFrameStart(mock_gles->GetProcTable());
-
-  calls = mock_gles->GetCapturedCalls();
-  std::vector<std::string> expected_b = {"glGetQueryObjectuivEXT",
-                                         "glGetQueryObjectui64vEXT",
-                                         "glDeleteQueriesEXT"};
-  for (auto i = 0; i < 3; i++) {
-    EXPECT_EQ(calls[i], expected_b[i]);
-  }
 }
 
 #endif  // IMPELLER_DEBUG

engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles.cc

@@ -19,18 +19,9 @@ static std::mutex g_test_lock;
 
 static std::weak_ptr<MockGLES> g_mock_gles;
 
-static ProcTableGLES::Resolver g_resolver;
+static std::vector<const char*> g_extensions;
 
-static std::vector<const unsigned char*> g_extensions;
-
-static const unsigned char* g_version;
-
-// Has friend visibility into MockGLES to record calls.
-void RecordGLCall(const char* name) {
-  if (auto mock_gles = g_mock_gles.lock()) {
-    mock_gles->RecordCall(name);
-  }
-}
+static const char* g_version;
 
 template <typename T, typename U>
 struct CheckSameSignature : std::false_type {};
@@ -41,22 +32,34 @@ struct CheckSameSignature<Ret(Args...), Ret(Args...)> : std::true_type {};
 // This is a stub function that does nothing/records nothing.
 void doNothing() {}
 
-auto const kMockVendor = (unsigned char*)"MockGLES";
-const auto kMockShadingLanguageVersion = (unsigned char*)"GLSL ES 1.0";
-auto const kExtensions = std::vector<const unsigned char*>{
-    (unsigned char*)"GL_KHR_debug"  //
+auto const kMockVendor = "MockGLES";
+const auto kMockShadingLanguageVersion = "GLSL ES 1.0";
+auto const kExtensions = std::vector<const char*>{
+    "GL_KHR_debug"  //
 };
 
+namespace {
+template <typename Func, typename... Args>
+void CallMockMethod(Func func, Args&&... args) {
+  if (auto mock_gles = g_mock_gles.lock()) {
+    if (mock_gles->GetImpl()) {
+      (mock_gles->GetImpl()->*func)(std::forward<Args>(args)...);
+    }
+  }
+}
+}  // namespace
+
 const unsigned char* mockGetString(GLenum name) {
   switch (name) {
     case GL_VENDOR:
-      return kMockVendor;
+      return reinterpret_cast<const unsigned char*>(kMockVendor);
     case GL_VERSION:
-      return g_version;
+      return reinterpret_cast<const unsigned char*>(g_version);
     case GL_SHADING_LANGUAGE_VERSION:
-      return kMockShadingLanguageVersion;
+      return reinterpret_cast<const unsigned char*>(
+          kMockShadingLanguageVersion);
     default:
-      return (unsigned char*)"";
+      return reinterpret_cast<const unsigned char*>("");
   }
 }
 
@@ -66,9 +69,9 @@ static_assert(CheckSameSignature<decltype(mockGetString),  //
 const unsigned char* mockGetStringi(GLenum name, GLuint index) {
   switch (name) {
     case GL_EXTENSIONS:
-      return g_extensions[index];
+      return reinterpret_cast<const unsigned char*>(g_extensions[index]);
     default:
-      return (unsigned char*)"";
+      return reinterpret_cast<const unsigned char*>("");
   }
 }
 
@@ -102,89 +105,73 @@ GLenum mockGetError() {
 static_assert(CheckSameSignature<decltype(mockGetError),  //
                                  decltype(glGetError)>::value);
 
-void mockPopDebugGroupKHR() {
-  RecordGLCall("PopDebugGroupKHR");
-}
+void mockPopDebugGroupKHR() {}
 
 static_assert(CheckSameSignature<decltype(mockPopDebugGroupKHR),  //
                                  decltype(glPopDebugGroupKHR)>::value);
 
 void mockPushDebugGroupKHR(GLenum source,
                            GLuint id,
                            GLsizei length,
-                           const GLchar* message) {
-  RecordGLCall("PushDebugGroupKHR");
-}
+                           const GLchar* message) {}
 
 static_assert(CheckSameSignature<decltype(mockPushDebugGroupKHR),  //
                                  decltype(glPushDebugGroupKHR)>::value);
 
 void mockGenQueriesEXT(GLsizei n, GLuint* ids) {
-  RecordGLCall("glGenQueriesEXT");
-  for (auto i = 0; i < n; i++) {
-    ids[i] = i + 1;
-  }
+  CallMockMethod(&IMockGLESImpl::GenQueriesEXT, n, ids);
 }
 
 static_assert(CheckSameSignature<decltype(mockGenQueriesEXT),  //
                                  decltype(glGenQueriesEXT)>::value);
 
 void mockBeginQueryEXT(GLenum target, GLuint id) {
-  RecordGLCall("glBeginQueryEXT");
+  CallMockMethod(&IMockGLESImpl::BeginQueryEXT, target, id);
 }
 
 static_assert(CheckSameSignature<decltype(mockBeginQueryEXT),  //
                                  decltype(glBeginQueryEXT)>::value);
 
 void mockEndQueryEXT(GLuint id) {
-  RecordGLCall("glEndQueryEXT");
+  CallMockMethod(&IMockGLESImpl::EndQueryEXT, id);
 }
 
 static_assert(CheckSameSignature<decltype(mockEndQueryEXT),  //
                                  decltype(glEndQueryEXT)>::value);
 
 void mockGetQueryObjectuivEXT(GLuint id, GLenum target, GLuint* result) {
-  RecordGLCall("glGetQueryObjectuivEXT");
-  *result = GL_TRUE;
+  CallMockMethod(&IMockGLESImpl::GetQueryObjectuivEXT, id, target, result);
 }
 
 static_assert(CheckSameSignature<decltype(mockGetQueryObjectuivEXT),  //
                                  decltype(glGetQueryObjectuivEXT)>::value);
 
 void mockGetQueryObjectui64vEXT(GLuint id, GLenum target, GLuint64* result) {
-  RecordGLCall("glGetQueryObjectui64vEXT");
-  *result = 1000u;
+  CallMockMethod(&IMockGLESImpl::GetQueryObjectui64vEXT, id, target, result);
 }
 
 static_assert(CheckSameSignature<decltype(mockGetQueryObjectui64vEXT),  //
                                  decltype(glGetQueryObjectui64vEXT)>::value);
 
 void mockDeleteQueriesEXT(GLsizei size, const GLuint* queries) {
-  RecordGLCall("glDeleteQueriesEXT");
+  CallMockMethod(&IMockGLESImpl::DeleteQueriesEXT, size, queries);
 }
 
 void mockDeleteTextures(GLsizei size, const GLuint* queries) {
-  RecordGLCall("glDeleteTextures");
+  CallMockMethod(&IMockGLESImpl::DeleteTextures, size, queries);
 }
 
 static_assert(CheckSameSignature<decltype(mockDeleteQueriesEXT),  //
                                  decltype(glDeleteQueriesEXT)>::value);
 
 void mockUniform1fv(GLint location, GLsizei count, const GLfloat* value) {
-  RecordGLCall("glUniform1fv");
+  CallMockMethod(&IMockGLESImpl::Uniform1fv, location, count, value);
 }
 static_assert(CheckSameSignature<decltype(mockUniform1fv),  //
                                  decltype(glUniform1fv)>::value);
 
 void mockGenTextures(GLsizei n, GLuint* textures) {
-  RecordGLCall("glGenTextures");
-  if (auto mock_gles = g_mock_gles.lock()) {
-    std::optional<uint64_t> next_texture;
-    std::swap(mock_gles->next_texture_, next_texture);
-    if (next_texture.has_value()) {
-      textures[0] = next_texture.value();
-    }
-  }
+  CallMockMethod(&IMockGLESImpl::GenTextures, n, textures);
 }
 
 static_assert(CheckSameSignature<decltype(mockGenTextures),  //
@@ -194,20 +181,35 @@ void mockObjectLabelKHR(GLenum identifier,
                         GLuint name,
                         GLsizei length,
                         const GLchar* label) {
-  RecordGLCall("glObjectLabelKHR");
+  CallMockMethod(&IMockGLESImpl::ObjectLabelKHR, identifier, name, length,
+                 label);
 }
 static_assert(CheckSameSignature<decltype(mockObjectLabelKHR),  //
                                  decltype(glObjectLabelKHR)>::value);
 
+// static
+std::shared_ptr<MockGLES> MockGLES::Init(
+    std::unique_ptr<MockGLESImpl> impl,
+    const std::optional<std::vector<const char*>>& extensions) {
+  FML_CHECK(g_test_lock.try_lock())
+      << "MockGLES is already being used by another test.";
+  g_extensions = extensions.value_or(kExtensions);
+  g_version = "OpenGL ES 3.0";
+  auto mock_gles = std::shared_ptr<MockGLES>(new MockGLES());
+  mock_gles->impl_ = std::move(impl);
+  g_mock_gles = mock_gles;
+  return mock_gles;
+}
+
 std::shared_ptr<MockGLES> MockGLES::Init(
-    const std::optional<std::vector<const unsigned char*>>& extensions,
+    const std::optional<std::vector<const char*>>& extensions,
     const char* version_string,
     ProcTableGLES::Resolver resolver) {
   // If we cannot obtain a lock, MockGLES is already being used elsewhere.
   FML_CHECK(g_test_lock.try_lock())
       << "MockGLES is already being used by another test.";
-  g_version = (unsigned char*)version_string;
   g_extensions = extensions.value_or(kExtensions);
+  g_version = version_string;
   auto mock_gles = std::shared_ptr<MockGLES>(new MockGLES(std::move(resolver)));
   g_mock_gles = mock_gles;
   return mock_gles;