-
Notifications
You must be signed in to change notification settings - Fork 6k
Add service protocol method to facilitate getting snapshots #32628
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2524,6 +2524,49 @@ TEST_F(ShellTest, OnServiceProtocolEstimateRasterCacheMemoryWorks) { | |
DestroyShell(std::move(shell)); | ||
} | ||
|
||
// ktz | ||
TEST_F(ShellTest, OnServiceProtocolRenderFrameWithRasterStatsWorks) { | ||
auto settings = CreateSettingsForFixture(); | ||
std::unique_ptr<Shell> shell = CreateShell(settings); | ||
|
||
// Create the surface needed by rasterizer | ||
PlatformViewNotifyCreated(shell.get()); | ||
|
||
auto configuration = RunConfiguration::InferFromSettings(settings); | ||
configuration.SetEntrypoint("scene_with_red_box"); | ||
|
||
RunEngine(shell.get(), std::move(configuration)); | ||
PumpOneFrame(shell.get()); | ||
|
||
ServiceProtocol::Handler::ServiceProtocolMap empty_params; | ||
rapidjson::Document document; | ||
OnServiceProtocol( | ||
shell.get(), ServiceProtocolEnum::kRenderFrameWithRasterStats, | ||
shell->GetTaskRunners().GetRasterTaskRunner(), empty_params, &document); | ||
rapidjson::StringBuffer buffer; | ||
rapidjson::Writer<rapidjson::StringBuffer> writer(buffer); | ||
document.Accept(writer); | ||
|
||
// It would be better to parse out the json and check for the validity of | ||
// fields. Below checks approximate what needs to be checked, this can not be | ||
// an exact check since duration will not exactly match. | ||
std::string expected_json = | ||
"\"snapshot\":[137,80,78,71,13,10,26,10,0," | ||
"0,0,13,73,72,68,82,0,0,0,1,0,0,0,1,8,6,0,0,0,31,21,196,137,0,0,0,1,115," | ||
"82,71,66,0,174,206,28,233,0,0,0,4,115,66,73,84,8,8,8,8,124,8,100,136,0," | ||
"0,0,13,73,68,65,84,8,153,99,248,207,192,240,31,0,5,0,1,255,171,206,54," | ||
"137,0,0,0,0,73,69,78,68,174,66,96,130]"; | ||
std::string actual_json = buffer.GetString(); | ||
|
||
EXPECT_THAT(actual_json, ::testing::HasSubstr(expected_json)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are there JSON methods we could be using instead to validate fields? e.g. validate that snapshot is a list of integers, and validate that the timing field is present? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could maybe parse it out with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (I'm mainly worried that this test will break if we move to a different rendering backend or the rendering backend slightly changes things and it'll take more work to validate) |
||
EXPECT_THAT(actual_json, | ||
::testing::HasSubstr("{\"type\":\"RenderFrameWithRasterStats\"")); | ||
EXPECT_THAT(actual_json, ::testing::HasSubstr("\"duration_micros\"")); | ||
|
||
PlatformViewNotifyDestroyed(shell.get()); | ||
DestroyShell(std::move(shell)); | ||
} | ||
|
||
// TODO(https://github.com/flutter/flutter/issues/100273): Disabled due to | ||
// flakiness. | ||
// TODO(https://github.com/flutter/flutter/issues/100299): Fix it when | ||
|
@@ -2913,8 +2956,8 @@ TEST_F(ShellTest, AssetManagerMultiSubdir) { | |
|
||
std::vector<std::string> filenames = { | ||
"bad0", | ||
"notgood", // this is to make sure the pattern (.*)good(.*) only matches | ||
// things in the subdirectory | ||
"notgood", // this is to make sure the pattern (.*)good(.*) only | ||
// matches things in the subdirectory | ||
}; | ||
|
||
std::vector<std::string> subdir_filenames = { | ||
|
@@ -2989,8 +3032,9 @@ TEST_F(ShellTest, Spawn) { | |
// Fulfill native function for the second Shell's entrypoint. | ||
fml::CountDownLatch second_latch(2); | ||
AddNativeCallback( | ||
// The Dart native function names aren't very consistent but this is just | ||
// the native function name of the second vm entrypoint in the fixture. | ||
// The Dart native function names aren't very consistent but this is | ||
// just the native function name of the second vm entrypoint in the | ||
// fixture. | ||
"NotifyNative", | ||
CREATE_NATIVE_ENTRY([&](auto args) { second_latch.CountDown(); })); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider adding a doc comment explaining how this was generated and what might cause it to change (and what an acceptable change would be).