From 8fa289f93a513721f725b208b1ac079f0d82be06 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 24 Sep 2020 14:58:58 -0700 Subject: [PATCH 1/2] Avoid sending a 0 DPR to framework --- lib/ui/window/viewport_metrics.cc | 11 +------ shell/common/fixtures/shell_test.dart | 9 ++++++ shell/common/shell.cc | 6 ++++ shell/common/shell_unittests.cc | 45 +++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 10 deletions(-) diff --git a/lib/ui/window/viewport_metrics.cc b/lib/ui/window/viewport_metrics.cc index f642bd116966f..8db0996ab81eb 100644 --- a/lib/ui/window/viewport_metrics.cc +++ b/lib/ui/window/viewport_metrics.cc @@ -42,10 +42,6 @@ ViewportMetrics::ViewportMetrics(double p_device_pixel_ratio, physical_system_gesture_inset_bottom( p_physical_system_gesture_inset_bottom), physical_system_gesture_inset_left(p_physical_system_gesture_inset_left) { - // Ensure we don't have nonsensical dimensions. - FML_DCHECK(physical_width >= 0); - FML_DCHECK(physical_height >= 0); - FML_DCHECK(device_pixel_ratio > 0); } ViewportMetrics::ViewportMetrics(double p_device_pixel_ratio, @@ -53,11 +49,6 @@ ViewportMetrics::ViewportMetrics(double p_device_pixel_ratio, double p_physical_height) : device_pixel_ratio(p_device_pixel_ratio), physical_width(p_physical_width), - physical_height(p_physical_height) { - // Ensure we don't have nonsensical dimensions. - FML_DCHECK(physical_width >= 0); - FML_DCHECK(physical_height >= 0); - FML_DCHECK(device_pixel_ratio > 0); -} + physical_height(p_physical_height) {} } // namespace flutter diff --git a/shell/common/fixtures/shell_test.dart b/shell/common/fixtures/shell_test.dart index ceeee77a4e4f7..2675867fcd5cd 100644 --- a/shell/common/fixtures/shell_test.dart +++ b/shell/common/fixtures/shell_test.dart @@ -47,6 +47,15 @@ void onPointerDataPacketMain() { @pragma('vm:entry-point') void emptyMain() {} +@pragma('vm:entry-point') +void reportDevicePixelRatio() { + window.onMetricsChanged = () { + _reportDevicePixelRatio(window.devicePixelRatio); + }; +} + +void _reportDevicePixelRatio(double devicePixelRatio) native 'ReportDevicePixelRatio'; + @pragma('vm:entry-point') void dummyReportTimingsMain() { window.onReportTimings = (List timings) {}; diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 6bd27c260e82d..b23356648ace0 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -816,6 +816,12 @@ void Shell::OnPlatformViewSetViewportMetrics(const ViewportMetrics& metrics) { FML_DCHECK(is_setup_); FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); + if (metrics.device_pixel_ratio <= 0) { + FML_DLOG(ERROR) + << "Embedding reported a device pixel ratio of 0, ignoring update."; + return; + } + // This is the formula Android uses. // https://android.googlesource.com/platform/frameworks/base/+/master/libs/hwui/renderthread/CacheManager.cpp#41 size_t max_bytes = metrics.physical_width * metrics.physical_height * 12 * 4; diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index e780384f8ecb6..5f9e574723d44 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -2066,5 +2066,50 @@ TEST_F(ShellTest, DiscardLayerTreeOnResize) { DestroyShell(std::move(shell)); } +TEST_F(ShellTest, IgnoresZeroDPR) { + fml::AutoResetWaitableEvent latch; + double last_device_pixel_ratio; + auto native_report_device_pixel_ratio = [&](Dart_NativeArguments args) { + auto dpr_handle = Dart_GetNativeArgument(args, 0); + ASSERT_TRUE(Dart_IsDouble(dpr_handle)); + Dart_DoubleValue(dpr_handle, &last_device_pixel_ratio); + ASSERT_FALSE(last_device_pixel_ratio == 0.0); + latch.Signal(); + }; + + Settings settings = CreateSettingsForFixture(); + auto task_runner = CreateNewThread(); + TaskRunners task_runners("test", task_runner, task_runner, task_runner, + task_runner); + + AddNativeCallback("ReportDevicePixelRatio", + CREATE_NATIVE_ENTRY(native_report_device_pixel_ratio)); + + std::unique_ptr shell = + CreateShell(std::move(settings), std::move(task_runners)); + + auto configuration = RunConfiguration::InferFromSettings(settings); + configuration.SetEntrypoint("reportDevicePixelRatio"); + + RunEngine(shell.get(), std::move(configuration)); + + task_runner->PostTask([&]() { + shell->GetPlatformView()->SetViewportMetrics({0.0, 400, 200}); + task_runner->PostTask([&]() { + shell->GetPlatformView()->SetViewportMetrics({0.8, 400, 200}); + }); + }); + latch.Wait(); + ASSERT_EQ(last_device_pixel_ratio, 0.8); + latch.Reset(); + task_runner->PostTask([&]() { + shell->GetPlatformView()->SetViewportMetrics({1.2, 400, 200}); + }); + latch.Wait(); + ASSERT_EQ(last_device_pixel_ratio, 1.2); + + DestroyShell(std::move(shell), std::move(task_runners)); +} + } // namespace testing } // namespace flutter From 05e8cba9c9af5c0a64f05e62ddadd97b32a98d55 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 24 Sep 2020 15:34:29 -0700 Subject: [PATCH 2/2] check width and height as well --- shell/common/fixtures/shell_test.dart | 10 +++++--- shell/common/shell.cc | 8 +++++-- shell/common/shell_unittests.cc | 34 +++++++++++++++++++++++---- 3 files changed, 42 insertions(+), 10 deletions(-) diff --git a/shell/common/fixtures/shell_test.dart b/shell/common/fixtures/shell_test.dart index 2675867fcd5cd..75fad7e109283 100644 --- a/shell/common/fixtures/shell_test.dart +++ b/shell/common/fixtures/shell_test.dart @@ -48,13 +48,17 @@ void onPointerDataPacketMain() { void emptyMain() {} @pragma('vm:entry-point') -void reportDevicePixelRatio() { +void reportMetrics() { window.onMetricsChanged = () { - _reportDevicePixelRatio(window.devicePixelRatio); + _reportMetrics( + window.devicePixelRatio, + window.physicalSize.width, + window.physicalSize.height, + ); }; } -void _reportDevicePixelRatio(double devicePixelRatio) native 'ReportDevicePixelRatio'; +void _reportMetrics(double devicePixelRatio, double width, double height) native 'ReportMetrics'; @pragma('vm:entry-point') void dummyReportTimingsMain() { diff --git a/shell/common/shell.cc b/shell/common/shell.cc index b23356648ace0..7b4c641b4e76f 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -816,9 +816,13 @@ void Shell::OnPlatformViewSetViewportMetrics(const ViewportMetrics& metrics) { FML_DCHECK(is_setup_); FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); - if (metrics.device_pixel_ratio <= 0) { + if (metrics.device_pixel_ratio <= 0 || metrics.physical_width <= 0 || + metrics.physical_height <= 0) { FML_DLOG(ERROR) - << "Embedding reported a device pixel ratio of 0, ignoring update."; + << "Embedding reported invalid ViewportMetrics, ignoring update." + << "\nphysical_width: " << metrics.physical_width + << "\nphysical_height: " << metrics.physical_height + << "\ndevice_pixel_ratio: " << metrics.device_pixel_ratio; return; } diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 5f9e574723d44..2e708d0cbc612 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -2066,14 +2066,27 @@ TEST_F(ShellTest, DiscardLayerTreeOnResize) { DestroyShell(std::move(shell)); } -TEST_F(ShellTest, IgnoresZeroDPR) { +TEST_F(ShellTest, IgnoresInvalidMetrics) { fml::AutoResetWaitableEvent latch; double last_device_pixel_ratio; + double last_width; + double last_height; auto native_report_device_pixel_ratio = [&](Dart_NativeArguments args) { auto dpr_handle = Dart_GetNativeArgument(args, 0); ASSERT_TRUE(Dart_IsDouble(dpr_handle)); Dart_DoubleValue(dpr_handle, &last_device_pixel_ratio); ASSERT_FALSE(last_device_pixel_ratio == 0.0); + + auto width_handle = Dart_GetNativeArgument(args, 1); + ASSERT_TRUE(Dart_IsDouble(width_handle)); + Dart_DoubleValue(width_handle, &last_width); + ASSERT_FALSE(last_width == 0.0); + + auto height_handle = Dart_GetNativeArgument(args, 2); + ASSERT_TRUE(Dart_IsDouble(height_handle)); + Dart_DoubleValue(height_handle, &last_height); + ASSERT_FALSE(last_height == 0.0); + latch.Signal(); }; @@ -2082,31 +2095,42 @@ TEST_F(ShellTest, IgnoresZeroDPR) { TaskRunners task_runners("test", task_runner, task_runner, task_runner, task_runner); - AddNativeCallback("ReportDevicePixelRatio", + AddNativeCallback("ReportMetrics", CREATE_NATIVE_ENTRY(native_report_device_pixel_ratio)); std::unique_ptr shell = CreateShell(std::move(settings), std::move(task_runners)); auto configuration = RunConfiguration::InferFromSettings(settings); - configuration.SetEntrypoint("reportDevicePixelRatio"); + configuration.SetEntrypoint("reportMetrics"); RunEngine(shell.get(), std::move(configuration)); task_runner->PostTask([&]() { shell->GetPlatformView()->SetViewportMetrics({0.0, 400, 200}); task_runner->PostTask([&]() { - shell->GetPlatformView()->SetViewportMetrics({0.8, 400, 200}); + shell->GetPlatformView()->SetViewportMetrics({0.8, 0.0, 200}); + task_runner->PostTask([&]() { + shell->GetPlatformView()->SetViewportMetrics({0.8, 400, 0.0}); + task_runner->PostTask([&]() { + shell->GetPlatformView()->SetViewportMetrics({0.8, 400, 200.0}); + }); + }); }); }); latch.Wait(); ASSERT_EQ(last_device_pixel_ratio, 0.8); + ASSERT_EQ(last_width, 400.0); + ASSERT_EQ(last_height, 200.0); latch.Reset(); + task_runner->PostTask([&]() { - shell->GetPlatformView()->SetViewportMetrics({1.2, 400, 200}); + shell->GetPlatformView()->SetViewportMetrics({1.2, 600, 300}); }); latch.Wait(); ASSERT_EQ(last_device_pixel_ratio, 1.2); + ASSERT_EQ(last_width, 600.0); + ASSERT_EQ(last_height, 300.0); DestroyShell(std::move(shell), std::move(task_runners)); }