Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 88baf62

Browse files
authored
Move window state update to window realize callback (#47713)
## Description This moves the state update to only happen on realizing the window instead of at initialization time, based on [the comment from](flutter/flutter#137262 (comment)) @robert-ancell . ## Related Issues - flutter/flutter#137262 ## Tests - I tried to add tests, but it doesn't seem possible to create a view without an actual display connected (and making a mock of it defeats the purpose of the test). I'm happy to be proven wrong, though.
1 parent 8b490a9 commit 88baf62

File tree

8 files changed

+152
-14
lines changed

8 files changed

+152
-14
lines changed

ci/licenses_golden/licenses_flutter

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5053,6 +5053,7 @@ ORIGIN: ../../../flutter/shell/platform/linux/fl_view_accessible.cc + ../../../f
50535053
ORIGIN: ../../../flutter/shell/platform/linux/fl_view_accessible.h + ../../../flutter/LICENSE
50545054
ORIGIN: ../../../flutter/shell/platform/linux/fl_view_accessible_test.cc + ../../../flutter/LICENSE
50555055
ORIGIN: ../../../flutter/shell/platform/linux/fl_view_private.h + ../../../flutter/LICENSE
5056+
ORIGIN: ../../../flutter/shell/platform/linux/fl_view_test.cc + ../../../flutter/LICENSE
50565057
ORIGIN: ../../../flutter/shell/platform/linux/key_mapping.g.cc + ../../../flutter/LICENSE
50575058
ORIGIN: ../../../flutter/shell/platform/linux/key_mapping.h + ../../../flutter/LICENSE
50585059
ORIGIN: ../../../flutter/shell/platform/linux/public/flutter_linux/fl_basic_message_channel.h + ../../../flutter/LICENSE
@@ -7835,6 +7836,7 @@ FILE: ../../../flutter/shell/platform/linux/fl_view_accessible.cc
78357836
FILE: ../../../flutter/shell/platform/linux/fl_view_accessible.h
78367837
FILE: ../../../flutter/shell/platform/linux/fl_view_accessible_test.cc
78377838
FILE: ../../../flutter/shell/platform/linux/fl_view_private.h
7839+
FILE: ../../../flutter/shell/platform/linux/fl_view_test.cc
78387840
FILE: ../../../flutter/shell/platform/linux/key_mapping.g.cc
78397841
FILE: ../../../flutter/shell/platform/linux/key_mapping.h
78407842
FILE: ../../../flutter/shell/platform/linux/public/flutter_linux/fl_basic_message_channel.h

shell/platform/linux/BUILD.gn

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,10 @@ executable("flutter_linux_unittests") {
227227
"fl_texture_registrar_test.cc",
228228
"fl_value_test.cc",
229229
"fl_view_accessible_test.cc",
230+
"fl_view_test.cc",
230231
"testing/fl_test.cc",
232+
"testing/fl_test_gtk_logs.cc",
233+
"testing/fl_test_gtk_logs.h",
231234
"testing/mock_binary_messenger.cc",
232235
"testing/mock_binary_messenger_response_handle.cc",
233236
"testing/mock_engine.cc",

shell/platform/linux/fl_view.cc

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -247,8 +247,6 @@ static void on_pre_engine_restart_cb(FlEngine* engine, gpointer user_data) {
247247
g_clear_object(&self->scrolling_manager);
248248
init_keyboard(self);
249249
init_scrolling(self);
250-
self->window_state =
251-
gdk_window_get_state(gtk_widget_get_window(GTK_WIDGET(self)));
252250
}
253251

254252
// Implements FlPluginRegistry::get_registrar_for_plugin.
@@ -556,6 +554,8 @@ static void realize_cb(GtkWidget* widget) {
556554
self->window_state_cb_id =
557555
g_signal_connect(toplevel_window, "window-state-event",
558556
G_CALLBACK(window_state_event_cb), self);
557+
self->window_state =
558+
gdk_window_get_state(gtk_widget_get_window(toplevel_window));
559559

560560
g_signal_connect(toplevel_window, "delete-event",
561561
G_CALLBACK(window_delete_event_cb), self);
@@ -760,8 +760,6 @@ static void fl_view_class_init(FlViewClass* klass) {
760760

761761
static void fl_view_init(FlView* self) {
762762
gtk_widget_set_can_focus(GTK_WIDGET(self), TRUE);
763-
self->window_state = gdk_window_get_state(
764-
gtk_widget_get_window(gtk_widget_get_toplevel(GTK_WIDGET(self))));
765763
}
766764

767765
G_MODULE_EXPORT FlView* fl_view_new(FlDartProject* project) {
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#include "flutter/shell/platform/linux/public/flutter_linux/fl_view.h"
6+
#include "flutter/shell/platform/linux/testing/fl_test_gtk_logs.h"
7+
8+
#include "gtest/gtest.h"
9+
10+
TEST(FlViewTest, StateUpdateDoesNotHappenInInit) {
11+
flutter::testing::fl_ensure_gtk_init();
12+
g_autoptr(FlDartProject) project = fl_dart_project_new();
13+
g_autoptr(FlView) view = fl_view_new(project);
14+
// Check that creating a view doesn't try to query the window state in
15+
// initialization, causing a critical log to be issued.
16+
EXPECT_EQ(
17+
flutter::testing::fl_get_received_gtk_log_levels() & G_LOG_LEVEL_CRITICAL,
18+
(GLogLevelFlags)0x0);
19+
g_object_ref_sink(view);
20+
}
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#include "fl_test_gtk_logs.h"
6+
#include "gtest/gtest.h"
7+
8+
namespace flutter {
9+
namespace testing {
10+
11+
namespace {
12+
13+
bool gtk_initialized = false;
14+
GLogWriterFunc log_writer_cb = nullptr;
15+
GLogLevelFlags fl_received_log_levels = (GLogLevelFlags)0x0;
16+
17+
GLogWriterOutput log_writer(GLogLevelFlags log_level,
18+
const GLogField* fields,
19+
gsize n_fields,
20+
gpointer user_data) {
21+
fl_received_log_levels = (GLogLevelFlags)(log_level | fl_received_log_levels);
22+
if (log_writer_cb == nullptr) {
23+
return g_log_writer_default(log_level, fields, n_fields, user_data);
24+
}
25+
GLogWriterOutput result =
26+
log_writer_cb(log_level, fields, n_fields, user_data);
27+
if (result != G_LOG_WRITER_HANDLED) {
28+
return g_log_writer_default(log_level, fields, n_fields, user_data);
29+
}
30+
return result;
31+
}
32+
33+
} // namespace
34+
35+
void fl_ensure_gtk_init(GLogWriterFunc writer) {
36+
if (!gtk_initialized) {
37+
gtk_init(0, nullptr);
38+
g_log_set_writer_func(log_writer, nullptr, nullptr);
39+
gtk_initialized = true;
40+
}
41+
fl_reset_received_gtk_log_levels();
42+
log_writer_cb = writer;
43+
}
44+
45+
void fl_reset_received_gtk_log_levels() {
46+
fl_received_log_levels = (GLogLevelFlags)0x0;
47+
}
48+
49+
GLogLevelFlags fl_get_received_gtk_log_levels() {
50+
return fl_received_log_levels;
51+
}
52+
53+
} // namespace testing
54+
} // namespace flutter
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#ifndef FLUTTER_TESTING_TEST_GTK_LOGS_H_
6+
#define FLUTTER_TESTING_TEST_GTK_LOGS_H_
7+
8+
#include <gtk/gtk.h>
9+
10+
namespace flutter {
11+
namespace testing {
12+
13+
/**
14+
* Ensures that GTK has been initialized. If GTK has not been initialized, it
15+
* will be initialized using `gtk_init()`. It will also set the GTK log writer
16+
* function to monitor the log output, recording the log levels that have been
17+
* received in a bitfield accessible via {@link fl_get_received_gtk_log_levels}
18+
*
19+
* To retrieve the bitfield of recorded log levels, use
20+
* `fl_get_received_gtk_log_levels()`.
21+
*
22+
* @param[in] writer The custom log writer function to use. If `nullptr`, or it
23+
* returns G_LOG_WRITER_UNHANDLED, the default log writer function will be
24+
* called.
25+
*
26+
* @brief Ensures that GTK has been initialized and starts monitoring logs.
27+
*/
28+
void fl_ensure_gtk_init(GLogWriterFunc writer = nullptr);
29+
30+
/**
31+
* Resets the recorded GTK log levels to zero.
32+
*
33+
* @brief Resets the recorded log levels.
34+
*/
35+
void fl_reset_received_gtk_log_levels();
36+
37+
/**
38+
* Returns a bitfield containing the GTK log levels that have been seen since
39+
* the last time they were reset.
40+
*
41+
* @brief Returns the recorded log levels.
42+
*
43+
* @return A `GLogLevelFlags` bitfield representing the recorded log levels.
44+
*/
45+
GLogLevelFlags fl_get_received_gtk_log_levels();
46+
47+
} // namespace testing
48+
} // namespace flutter
49+
#endif // FLUTTER_TESTING_TEST_GTK_LOGS_H_

testing/BUILD.gn

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,11 @@ source_set("testing") {
5151

5252
sources = [ "run_all_unittests.cc" ]
5353

54+
if (is_linux) {
55+
# So that we can call gtk_init in main().
56+
configs += [ "//flutter/shell/platform/linux/config:gtk" ]
57+
}
58+
5459
public_deps = [ ":testing_lib" ]
5560
public_configs = [ ":dynamic_symbols" ]
5661
}

testing/run_tests.py

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -463,16 +463,23 @@ def make_test(name, flags=None, extra_env=None):
463463
make_test('flow_unittests', flags=repeat_flags + flow_flags),
464464
]
465465

466-
for test, flags, extra_env in unittests:
467-
run_engine_executable(
468-
build_dir,
469-
test,
470-
executable_filter,
471-
flags,
472-
coverage=coverage,
473-
extra_env=extra_env,
474-
gtest=True
475-
)
466+
build_name = os.path.basename(build_dir)
467+
try:
468+
if is_linux():
469+
xvfb.start_virtual_x(build_name, build_dir)
470+
for test, flags, extra_env in unittests:
471+
run_engine_executable(
472+
build_dir,
473+
test,
474+
executable_filter,
475+
flags,
476+
coverage=coverage,
477+
extra_env=extra_env,
478+
gtest=True
479+
)
480+
finally:
481+
if is_linux():
482+
xvfb.stop_virtual_x(build_name)
476483

477484
if is_mac():
478485
# flutter_desktop_darwin_unittests uses global state that isn't handled

0 commit comments

Comments
 (0)