Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,11 @@ group("flutter") {
"//flutter/third_party/txt:txt_unittests",
]

if (is_android) {
public_deps +=
[ "//flutter/shell/platform/android:flutter_shell_native_unittests" ]
}

# The accessibility library only supports Mac and Windows at the moment.
if (is_mac || is_win) {
public_deps +=
Expand Down
3 changes: 3 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,7 @@ FILE: ../../../flutter/shell/gpu/gpu_surface_vulkan_delegate.h
FILE: ../../../flutter/shell/platform/android/AndroidManifest.xml
FILE: ../../../flutter/shell/platform/android/android_context_gl.cc
FILE: ../../../flutter/shell/platform/android/android_context_gl.h
FILE: ../../../flutter/shell/platform/android/android_context_gl_unittests.cc
FILE: ../../../flutter/shell/platform/android/android_environment_gl.cc
FILE: ../../../flutter/shell/platform/android/android_environment_gl.h
FILE: ../../../flutter/shell/platform/android/android_exports.lst
Expand All @@ -779,6 +780,7 @@ FILE: ../../../flutter/shell/platform/android/android_image_generator.cc
FILE: ../../../flutter/shell/platform/android/android_image_generator.h
FILE: ../../../flutter/shell/platform/android/android_shell_holder.cc
FILE: ../../../flutter/shell/platform/android/android_shell_holder.h
FILE: ../../../flutter/shell/platform/android/android_shell_holder_unittests.cc
FILE: ../../../flutter/shell/platform/android/android_surface_gl.cc
FILE: ../../../flutter/shell/platform/android/android_surface_gl.h
FILE: ../../../flutter/shell/platform/android/android_surface_software.cc
Expand All @@ -795,6 +797,7 @@ FILE: ../../../flutter/shell/platform/android/external_view_embedder/surface_poo
FILE: ../../../flutter/shell/platform/android/external_view_embedder/surface_pool_unittests.cc
FILE: ../../../flutter/shell/platform/android/flutter_main.cc
FILE: ../../../flutter/shell/platform/android/flutter_main.h
FILE: ../../../flutter/shell/platform/android/flutter_shell_native_unittests.cc
FILE: ../../../flutter/shell/platform/android/io/flutter/FlutterInjector.java
FILE: ../../../flutter/shell/platform/android/io/flutter/Log.java
FILE: ../../../flutter/shell/platform/android/io/flutter/app/FlutterActivity.java
Expand Down
2 changes: 1 addition & 1 deletion shell/common/platform_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
#include <functional>
#include <memory>

#include "flow/embedded_views.h"
#include "flutter/common/graphics/texture.h"
#include "flutter/common/task_runners.h"
#include "flutter/flow/embedded_views.h"
#include "flutter/flow/surface.h"
#include "flutter/fml/macros.h"
#include "flutter/fml/mapping.h"
Expand Down
28 changes: 23 additions & 5 deletions shell/platform/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,30 @@ source_set("image_generator") {
]
}

shared_library("flutter_shell_native") {
visibility = [ ":*" ]
executable("flutter_shell_native_unittests") {
visibility = [ "*" ]
testonly = true
sources = [
"android_context_gl_unittests.cc",
"android_shell_holder_unittests.cc",
"flutter_shell_native_unittests.cc",
]
public_configs = [ "//flutter:config" ]
deps = [
":flutter_shell_native_src",
"//third_party/googletest:gmock",
"//third_party/googletest:gtest",
]
}

shared_library("flutter_shell_native") {
output_name = "flutter"
deps = [ ":flutter_shell_native_src" ]
ldflags = [ "-Wl,--version-script=" + rebase_path("android_exports.lst") ]
}

source_set("flutter_shell_native_src") {
visibility = [ ":*" ]

sources = [
"$root_build_dir/flutter_icu/icudtl.o",
Expand Down Expand Up @@ -69,7 +89,7 @@ shared_library("flutter_shell_native") {
"vsync_waiter_android.h",
]

deps = [
public_deps = [
":android_gpu_configuration",
":icudtl_object",
":image_generator",
Expand Down Expand Up @@ -100,8 +120,6 @@ shared_library("flutter_shell_native") {
"EGL",
"GLESv2",
]

ldflags = [ "-Wl,--version-script=" + rebase_path("android_exports.lst") ]
}

action("gen_android_build_config_java") {
Expand Down
16 changes: 10 additions & 6 deletions shell/platform/android/android_context_gl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -204,14 +204,18 @@ AndroidContextGL::~AndroidContextGL() {

std::unique_ptr<AndroidEGLSurface> AndroidContextGL::CreateOnscreenSurface(
fml::RefPtr<AndroidNativeWindow> window) const {
EGLDisplay display = environment_->Display();
if (window->IsFakeWindow()) {
return CreatePbufferSurface();
} else {
Comment on lines +207 to +209
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to create a PBuffer surface?

How confident can we be that we're testing what we even care about here?

Is there any possible way we could avoid this code in production code?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests in this PR are just very simple setup and teardown. This is more about setting up the infrastructure to make more interesting tests that do assert more things.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as the IsFakeWindow question, I looked into it. I didn't see a way to do it that wasn't a significant refactor. That's sometimes the problem when you add tests after the fact versus build them up with tests.

You could transfer creating the surface to the window, but that would leak egl calls outside of the AndroidContextGL. I'm open to any suggestion I might have overlooked.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to create a PBuffer surface?

The code assumes a surface, it will crash if you don't have one. We don't have a window so our only option is to make a fake pbuffer surface.

EGLDisplay display = environment_->Display();

const EGLint attribs[] = {EGL_NONE};
const EGLint attribs[] = {EGL_NONE};

EGLSurface surface = eglCreateWindowSurface(
display, config_, reinterpret_cast<EGLNativeWindowType>(window->handle()),
attribs);
return std::make_unique<AndroidEGLSurface>(surface, display, context_);
EGLSurface surface = eglCreateWindowSurface(
display, config_,
reinterpret_cast<EGLNativeWindowType>(window->handle()), attribs);
return std::make_unique<AndroidEGLSurface>(surface, display, context_);
}
}

std::unique_ptr<AndroidEGLSurface> AndroidContextGL::CreateOffscreenSurface()
Expand Down
11 changes: 11 additions & 0 deletions shell/platform/android/android_context_gl_unittests.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#include <memory>
#include "flutter/shell/platform/android/android_context_gl.h"
#include "flutter/shell/platform/android/android_environment_gl.h"
#include "gtest/gtest.h"

TEST(AndroidContextGl, Create) {
auto environment = fml::MakeRefCounted<flutter::AndroidEnvironmentGL>();
auto context = std::make_unique<flutter::AndroidContextGL>(
flutter::AndroidRenderingAPI::kOpenGLES, environment);
EXPECT_NE(context.get(), nullptr);
}
70 changes: 70 additions & 0 deletions shell/platform/android/android_shell_holder_unittests.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
#include <memory>
#include "flutter/shell/platform/android/android_shell_holder.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"

namespace flutter {
namespace testing {
namespace {
class MockPlatformViewAndroidJNI : public PlatformViewAndroidJNI {
MOCK_METHOD2(FlutterViewHandlePlatformMessage,
void(std::unique_ptr<flutter::PlatformMessage> message,
int responseId));
MOCK_METHOD2(FlutterViewHandlePlatformMessageResponse,
void(int responseId, std::unique_ptr<fml::Mapping> data));
MOCK_METHOD3(FlutterViewUpdateSemantics,
void(std::vector<uint8_t> buffer,
std::vector<std::string> strings,
std::vector<std::vector<uint8_t>> string_attribute_args));
MOCK_METHOD2(FlutterViewUpdateCustomAccessibilityActions,
void(std::vector<uint8_t> actions_buffer,
std::vector<std::string> strings));
MOCK_METHOD0(FlutterViewOnFirstFrame, void());
MOCK_METHOD0(FlutterViewOnPreEngineRestart, void());
MOCK_METHOD2(SurfaceTextureAttachToGLContext,
void(JavaLocalRef surface_texture, int textureId));
MOCK_METHOD1(SurfaceTextureUpdateTexImage,
void(JavaLocalRef surface_texture));
MOCK_METHOD2(SurfaceTextureGetTransformMatrix,
void(JavaLocalRef surface_texture, SkMatrix& transform));
MOCK_METHOD1(SurfaceTextureDetachFromGLContext,
void(JavaLocalRef surface_texture));
MOCK_METHOD8(FlutterViewOnDisplayPlatformView,
void(int view_id,
int x,
int y,
int width,
int height,
int viewWidth,
int viewHeight,
MutatorsStack mutators_stack));
MOCK_METHOD5(FlutterViewDisplayOverlaySurface,
void(int surface_id, int x, int y, int width, int height));
MOCK_METHOD0(FlutterViewBeginFrame, void());
MOCK_METHOD0(FlutterViewEndFrame, void());
MOCK_METHOD0(FlutterViewCreateOverlaySurface,
std::unique_ptr<PlatformViewAndroidJNI::OverlayMetadata>());
MOCK_METHOD0(FlutterViewDestroyOverlaySurfaces, void());
MOCK_METHOD1(FlutterViewComputePlatformResolvedLocale,
std::unique_ptr<std::vector<std::string>>(
std::vector<std::string> supported_locales_data));
MOCK_METHOD0(GetDisplayRefreshRate, double());
MOCK_METHOD1(RequestDartDeferredLibrary, bool(int loading_unit_id));
};
} // namespace

TEST(AndroidShellHolder, Create) {
Settings settings;
settings.enable_software_rendering = false;
auto jni = std::make_shared<MockPlatformViewAndroidJNI>();
auto holder = std::make_unique<AndroidShellHolder>(
settings, jni, /*is_background_view=*/false);
EXPECT_NE(holder.get(), nullptr);
EXPECT_TRUE(holder->IsValid());
EXPECT_NE(holder->GetPlatformView().get(), nullptr);
auto window =
fml::MakeRefCounted<AndroidNativeWindow>(nullptr, /*is_offscreen=*/true);
holder->GetPlatformView()->NotifyCreated(window);
}
} // namespace testing
} // namespace flutter
6 changes: 6 additions & 0 deletions shell/platform/android/flutter_shell_native_unittests.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include "gtest/gtest.h"

int main(int argc, char* argv[]) {
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
}
6 changes: 5 additions & 1 deletion shell/platform/android/surface/android_native_window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@

namespace flutter {

AndroidNativeWindow::AndroidNativeWindow(Handle window) : window_(window) {}
AndroidNativeWindow::AndroidNativeWindow(Handle window, bool is_fake_window)
: window_(window), is_fake_window_(is_fake_window) {}

AndroidNativeWindow::AndroidNativeWindow(Handle window)
: AndroidNativeWindow(window, /*is_fake_window=*/false) {}

AndroidNativeWindow::~AndroidNativeWindow() {
if (window_ != nullptr) {
Expand Down
7 changes: 7 additions & 0 deletions shell/platform/android/surface/android_native_window.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,20 @@ class AndroidNativeWindow

SkISize GetSize() const;

/// Returns true when this AndroidNativeWindow is not backed by a real window
/// (used for testing).
bool IsFakeWindow() const { return is_fake_window_; }

private:
Handle window_;
const bool is_fake_window_;

/// Creates a native window with the given handle. Handle ownership is assumed
/// by this instance of the native window.
explicit AndroidNativeWindow(Handle window);

explicit AndroidNativeWindow(Handle window, bool is_fake_window);

~AndroidNativeWindow();

FML_FRIEND_MAKE_REF_COUNTED(AndroidNativeWindow);
Expand Down
12 changes: 12 additions & 0 deletions testing/run_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,14 @@ def RunJavaTests(filter, android_variant='android_debug_unopt'):
RunCmd(command, cwd=test_runner_dir, env=env)


def RunAndroidTests(android_variant='android_debug_unopt'):
test_runner_name = 'flutter_shell_native_unittests'
tests_path = os.path.join(out_dir, android_variant, test_runner_name)
remote_path = '/data/local/tmp'
remote_tests_path = os.path.join(remote_path, test_runner_name)
RunCmd(['adb', 'push', tests_path, remote_path], cwd=buildroot_dir)
RunCmd(['adb', 'shell', remote_tests_path])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this validate failure or pass?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adb shell forwards the exit code which RunCmd grabs.


def RunObjcTests(ios_variant='ios_debug_sim_unopt', test_filter=None):
"""Runs Objective-C XCTest unit tests for the iOS embedding"""
AssertExpectedXcodeVersion()
Expand Down Expand Up @@ -592,6 +600,10 @@ def main():
java_filter = None
RunJavaTests(java_filter, args.android_variant)

if 'android' in types:
assert not IsWindows(), "Android engine files can't be compiled on Windows."
RunAndroidTests(args.android_variant)

if 'objc' in types:
assert IsMac(), "iOS embedding tests can only be run on macOS."
RunObjcTests(args.ios_variant, args.objc_filter)
Expand Down