Skip to content

Commit 61d7919

Browse files
fzyzcjyschwa423
authored andcommitted
Eliminate duplicated code when dealing with pointer data (flutter#36822)
* add methods * add empty test * add unit test * Update licenses_flutter * add const * size -> get length * add boundary checks * add tests * remove death test since it says "Death tests use fork(), which is unsafe particularly in a threaded context."
1 parent ce4ed81 commit 61d7919

File tree

9 files changed

+94
-34
lines changed

9 files changed

+94
-34
lines changed

ci/licenses_golden/licenses_flutter

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1781,6 +1781,7 @@ FILE: ../../../flutter/lib/ui/window/pointer_data_packet.h
17811781
FILE: ../../../flutter/lib/ui/window/pointer_data_packet_converter.cc
17821782
FILE: ../../../flutter/lib/ui/window/pointer_data_packet_converter.h
17831783
FILE: ../../../flutter/lib/ui/window/pointer_data_packet_converter_unittests.cc
1784+
FILE: ../../../flutter/lib/ui/window/pointer_data_packet_unittests.cc
17841785
FILE: ../../../flutter/lib/ui/window/viewport_metrics.cc
17851786
FILE: ../../../flutter/lib/ui/window/viewport_metrics.h
17861787
FILE: ../../../flutter/lib/ui/window/window.cc

lib/ui/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,7 @@ if (enable_unittests) {
239239
"window/platform_message_response_dart_port_unittests.cc",
240240
"window/platform_message_response_dart_unittests.cc",
241241
"window/pointer_data_packet_converter_unittests.cc",
242+
"window/pointer_data_packet_unittests.cc",
242243
]
243244

244245
deps = [

lib/ui/window/pointer_data_packet.cc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// found in the LICENSE file.
44

55
#include "flutter/lib/ui/window/pointer_data_packet.h"
6+
#include "flutter/fml/logging.h"
67

78
#include <cstring>
89

@@ -17,7 +18,19 @@ PointerDataPacket::PointerDataPacket(uint8_t* data, size_t num_bytes)
1718
PointerDataPacket::~PointerDataPacket() = default;
1819

1920
void PointerDataPacket::SetPointerData(size_t i, const PointerData& data) {
21+
FML_DCHECK(i < GetLength());
2022
memcpy(&data_[i * sizeof(PointerData)], &data, sizeof(PointerData));
2123
}
2224

25+
PointerData PointerDataPacket::GetPointerData(size_t i) const {
26+
FML_DCHECK(i < GetLength());
27+
PointerData result;
28+
memcpy(&result, &data_[i * sizeof(PointerData)], sizeof(PointerData));
29+
return result;
30+
}
31+
32+
size_t PointerDataPacket::GetLength() const {
33+
return data_.size() / sizeof(PointerData);
34+
}
35+
2336
} // namespace flutter

lib/ui/window/pointer_data_packet.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ class PointerDataPacket {
2020
~PointerDataPacket();
2121

2222
void SetPointerData(size_t i, const PointerData& data);
23+
PointerData GetPointerData(size_t i) const;
24+
size_t GetLength() const;
2325
const std::vector<uint8_t>& data() const { return data_; }
2426

2527
private:

lib/ui/window/pointer_data_packet_converter.cc

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,11 @@ PointerDataPacketConverter::~PointerDataPacketConverter() = default;
1717

1818
std::unique_ptr<PointerDataPacket> PointerDataPacketConverter::Convert(
1919
std::unique_ptr<PointerDataPacket> packet) {
20-
size_t kBytesPerPointerData = kPointerDataFieldCount * kBytesPerField;
21-
auto buffer = packet->data();
22-
size_t buffer_length = buffer.size();
23-
2420
std::vector<PointerData> converted_pointers;
2521
// Converts each pointer data in the buffer and stores it in the
2622
// converted_pointers.
27-
for (size_t i = 0; i < buffer_length / kBytesPerPointerData; i++) {
28-
PointerData pointer_data;
29-
memcpy(&pointer_data, &buffer[i * kBytesPerPointerData],
30-
sizeof(PointerData));
23+
for (size_t i = 0; i < packet->GetLength(); i++) {
24+
PointerData pointer_data = packet->GetPointerData(i);
3125
ConvertPointerData(pointer_data, converted_pointers);
3226
}
3327

lib/ui/window/pointer_data_packet_converter_unittests.cc

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -133,14 +133,8 @@ void CreateSimulatedTrackpadGestureData(PointerData& data, // NOLINT
133133

134134
void UnpackPointerPacket(std::vector<PointerData>& output, // NOLINT
135135
std::unique_ptr<PointerDataPacket> packet) {
136-
size_t kBytesPerPointerData = kPointerDataFieldCount * kBytesPerField;
137-
auto buffer = packet->data();
138-
size_t buffer_length = buffer.size();
139-
140-
for (size_t i = 0; i < buffer_length / kBytesPerPointerData; i++) {
141-
PointerData pointer_data;
142-
memcpy(&pointer_data, &buffer[i * kBytesPerPointerData],
143-
sizeof(PointerData));
136+
for (size_t i = 0; i < packet->GetLength(); i++) {
137+
PointerData pointer_data = packet->GetPointerData(i);
144138
output.push_back(pointer_data);
145139
}
146140
packet.reset();
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
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/lib/ui/window/pointer_data.h"
6+
7+
#include <cstring>
8+
9+
#include "gtest/gtest.h"
10+
#include "pointer_data_packet.h"
11+
12+
namespace flutter {
13+
namespace testing {
14+
15+
void CreateSimpleSimulatedPointerData(PointerData& data, // NOLINT
16+
PointerData::Change change,
17+
int64_t device,
18+
double dx,
19+
double dy,
20+
int64_t buttons) {
21+
data.time_stamp = 0;
22+
data.change = change;
23+
data.kind = PointerData::DeviceKind::kTouch;
24+
data.signal_kind = PointerData::SignalKind::kNone;
25+
data.device = device;
26+
data.pointer_identifier = 0;
27+
data.physical_x = dx;
28+
data.physical_y = dy;
29+
data.physical_delta_x = 0.0;
30+
data.physical_delta_y = 0.0;
31+
data.buttons = buttons;
32+
data.obscured = 0;
33+
data.synthesized = 0;
34+
data.pressure = 0.0;
35+
data.pressure_min = 0.0;
36+
data.pressure_max = 0.0;
37+
data.distance = 0.0;
38+
data.distance_max = 0.0;
39+
data.size = 0.0;
40+
data.radius_major = 0.0;
41+
data.radius_minor = 0.0;
42+
data.radius_min = 0.0;
43+
data.radius_max = 0.0;
44+
data.orientation = 0.0;
45+
data.tilt = 0.0;
46+
data.platformData = 0;
47+
data.scroll_delta_x = 0.0;
48+
data.scroll_delta_y = 0.0;
49+
}
50+
51+
TEST(PointerDataPacketTest, CanGetPointerData) {
52+
auto packet = std::make_unique<PointerDataPacket>(1);
53+
PointerData data;
54+
CreateSimpleSimulatedPointerData(data, PointerData::Change::kAdd, 1, 2.0, 3.0,
55+
4);
56+
packet->SetPointerData(0, data);
57+
58+
PointerData data_recovered = packet->GetPointerData(0);
59+
ASSERT_EQ(data_recovered.physical_x, 2.0);
60+
ASSERT_EQ(data_recovered.physical_y, 3.0);
61+
}
62+
63+
TEST(PointerDataPacketTest, CanGetLength) {
64+
auto packet = std::make_unique<PointerDataPacket>(6);
65+
ASSERT_EQ(packet->GetLength(), (size_t)6);
66+
}
67+
68+
} // namespace testing
69+
} // namespace flutter

shell/platform/fuchsia/flutter/platform_view_unittest.cc

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -379,15 +379,8 @@ std::string ToString(const fml::Mapping& mapping) {
379379
// Stolen from pointer_data_packet_converter_unittests.cc.
380380
void UnpackPointerPacket(std::vector<flutter::PointerData>& output, // NOLINT
381381
std::unique_ptr<flutter::PointerDataPacket> packet) {
382-
size_t kBytesPerPointerData =
383-
flutter::kPointerDataFieldCount * flutter::kBytesPerField;
384-
auto buffer = packet->data();
385-
size_t buffer_length = buffer.size();
386-
387-
for (size_t i = 0; i < buffer_length / kBytesPerPointerData; i++) {
388-
flutter::PointerData pointer_data;
389-
memcpy(&pointer_data, &buffer[i * kBytesPerPointerData],
390-
sizeof(flutter::PointerData));
382+
for (size_t i = 0; i < packet->GetLength(); i++) {
383+
flutter::PointerData pointer_data = packet->GetPointerData(i);
391384
output.push_back(pointer_data);
392385
}
393386
packet.reset();

shell/platform/fuchsia/flutter/tests/flatland_platform_view_unittest.cc

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -484,15 +484,8 @@ std::string ToString(const fml::Mapping& mapping) {
484484
// Stolen from pointer_data_packet_converter_unittests.cc.
485485
void UnpackPointerPacket(std::vector<flutter::PointerData>& output, // NOLINT
486486
std::unique_ptr<flutter::PointerDataPacket> packet) {
487-
size_t kBytesPerPointerData =
488-
flutter::kPointerDataFieldCount * flutter::kBytesPerField;
489-
auto buffer = packet->data();
490-
size_t buffer_length = buffer.size();
491-
492-
for (size_t i = 0; i < buffer_length / kBytesPerPointerData; i++) {
493-
flutter::PointerData pointer_data;
494-
memcpy(&pointer_data, &buffer[i * kBytesPerPointerData],
495-
sizeof(flutter::PointerData));
487+
for (size_t i = 0; i < packet->GetLength(); i++) {
488+
flutter::PointerData pointer_data = packet->GetPointerData(i);
496489
output.push_back(pointer_data);
497490
}
498491
packet.reset();

0 commit comments

Comments
 (0)