-
Notifications
You must be signed in to change notification settings - Fork 6k
[fuchsia][scenic] Accept Uint64 values in timestamp for pointerinjector. #34888
Conversation
Uint64 type values should be acceptable in the timestamp field of the platform message for pointerinjector. Test: ffx test run "fuchsia-pkg://fuchsia.com/flutter_runner_tests#meta/flutter_runner_tests.cm"
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
@@ -129,7 +129,7 @@ bool PointerInjectorDelegate::HandlePlatformMessage( | |||
} | |||
|
|||
auto timestamp = args.FindMember("timestamp"); | |||
if (!timestamp->value.IsInt()) { | |||
if (!timestamp->value.IsInt() && !timestamp->value.IsUint64()) { |
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.
Any unit test we can add for this?
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.
We already have some unittests in pointerinjector_delegate_unittest.cc which check this code path.
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.
Actually those were added as part of the previous PR (#34692). This was a minor bug which I discovered while doing some manual testing on estelle.
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.
I see, thanks, sorry for not checking first.
Uint64 type values should be acceptable in the timestamp field of the
platform message for pointerinjector.
Test: ffx test run "fuchsia-pkg://fuchsia.com/flutter_runner_tests#meta/flutter_runner_tests.cm"
Manual Testing
Deployed changes on Smart display (estelle)
Manually verified youtube was working through swipe and taps
Manually verified Duo is working through swipe and taps