summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHarry Cutts <hcutts@google.com>2023-02-27 11:16:41 +0000
committerHarry Cutts <hcutts@google.com>2023-02-27 11:22:26 +0000
commit37db0484f7bf99225c03eb279cfe9c0b270b9bc7 (patch)
tree9c0b07e112d194b628f8ea91cc1e47cea3f2ebb1
parent402ba203f42ea10e06eb7b846e99a64bb8465021 (diff)
parent3059225019e295de1138574a16c0102c6845fefb (diff)
downloadlibchrome-gestures-37db0484f7bf99225c03eb279cfe9c0b270b9bc7.tar.gz
Merge leak fix and logging changes from upstream
This also pulls in some improvements to the way properties are initialized, potentially preventing crashes (though not ones that have been observed on Android yet). Bug: 266848195 Bug: 251196347 Test: TreeHugger Change-Id: I78ff5fb20f2c5dfe81435b8f41168af60e88b665
-rw-r--r--METADATA.android4
-rw-r--r--include/iir_filter_interpreter.h4
-rw-r--r--include/list.h2
-rw-r--r--include/logging.h13
-rw-r--r--include/prop_registry.h69
-rw-r--r--src/gestures.cc2
-rw-r--r--src/iir_filter_interpreter.cc21
-rw-r--r--src/prop_registry.cc12
-rw-r--r--src/prop_registry_unittest.cc7
9 files changed, 72 insertions, 62 deletions
diff --git a/METADATA.android b/METADATA.android
index d9478a9..00233ca 100644
--- a/METADATA.android
+++ b/METADATA.android
@@ -6,7 +6,7 @@ third_party {
type: GIT
value: "https://chromium.googlesource.com/chromiumos/platform/gestures/"
}
- version: "ebe1f3fed44b84cc60294d76a680bb741876b3bd"
- last_upgrade_date { year: 2023 month: 02 day: 10 }
+ version: "3059225019e295de1138574a16c0102c6845fefb"
+ last_upgrade_date { year: 2023 month: 02 day: 27 }
license_type: NOTICE
}
diff --git a/include/iir_filter_interpreter.h b/include/iir_filter_interpreter.h
index 156a331..db940b8 100644
--- a/include/iir_filter_interpreter.h
+++ b/include/iir_filter_interpreter.h
@@ -79,9 +79,11 @@ class IirFilterInterpreter : public FilterInterpreter, public PropertyDelegate {
virtual void DoubleWasWritten(DoubleProperty* prop);
private:
- // Whether IIR filter should be used. Put as a member varible for
+ // Whether IIR filter should be used. Put as a member variable for
// unittest purpose.
bool using_iir_;
+
+ // Sync state history information
std::map<short, IoHistory> histories_;
// y[0] = b[0]*x[0] + b[1]*x[1] + b[2]*x[2] + b[3]*x[3]
diff --git a/include/list.h b/include/list.h
index 8e401bf..d5d94c4 100644
--- a/include/list.h
+++ b/include/list.h
@@ -49,7 +49,7 @@ class List {
virtual void DeleteAll() {
while (!Empty())
- PopFront();
+ delete PopFront();
}
Elt* Head() const { return sentinel_.next_; }
diff --git a/include/logging.h b/include/logging.h
index 4ae7bf6..31471c5 100644
--- a/include/logging.h
+++ b/include/logging.h
@@ -30,12 +30,21 @@
} while(false)
#define Log(format, ...) \
- gestures_log(GESTURES_LOG_INFO, "INFO:%s:%d:" format "\n", \
+ gestures_log(GESTURES_LOG_INFO, "INFO:%s:%d: " format "\n", \
__FILE__, __LINE__, ## __VA_ARGS__)
#define Err(format, ...) \
- gestures_log(GESTURES_LOG_ERROR, "ERROR:%s:%d:" format "\n", \
+ gestures_log(GESTURES_LOG_ERROR, "ERROR:%s:%d: " format "\n", \
__FILE__, __LINE__, ## __VA_ARGS__)
+#define ErrOnce(format, ...) \
+ do { \
+ static bool written = false; \
+ if (!written) { \
+ Err(format, ## __VA_ARGS__); \
+ written = true; \
+ } \
+ } while(false)
+
#define MTStatSample(key, value, timestamp) \
gestures_log(GESTURES_LOG_INFO, "MTStat:%f:%s:%s\n", \
(timestamp), (key), (value))
diff --git a/include/prop_registry.h b/include/prop_registry.h
index f2766e9..836b98b 100644
--- a/include/prop_registry.h
+++ b/include/prop_registry.h
@@ -46,9 +46,8 @@ class PropertyDelegate;
class Property {
public:
- Property(PropRegistry* parent, const char* name)
- : gprop_(NULL), parent_(parent), delegate_(NULL), name_(name) {}
- Property(PropRegistry* parent, const char* name, PropertyDelegate* delegate)
+ Property(PropRegistry* parent, const char* name,
+ PropertyDelegate* delegate)
: gprop_(NULL), parent_(parent), delegate_(delegate), name_(name) {}
virtual ~Property() {
@@ -60,6 +59,19 @@ class Property {
virtual void CreatePropImpl() = 0;
void DestroyProp();
+ // b/253585974
+ // delegate used to get passed as a constructor parameter but that
+ // led to a chance that the delegate was 'this' and setting the
+ // delegate to 'this' in the initializer list allowed the property
+ // creation to use 'this' before it was initialized. This could
+ // lead to unexpected behavior and if you were lucky to a crash.
+ //
+ // Now the delegate is always NULL on initilization of the property
+ // instance and after the delegate and property are completely
+ // created the user should set the delegate in the constructor
+ // body. This will allow access to this in a safe manner.
+ void SetDelegate(PropertyDelegate* delegate);
+
const char* name() { return name_; }
// Returns a newly allocated Value object
virtual Json::Value NewValue() const = 0;
@@ -89,13 +101,8 @@ class Property {
class BoolProperty : public Property {
public:
- BoolProperty(PropRegistry* reg, const char* name, GesturesPropBool val)
- : Property(reg, name), val_(val) {
- if (parent_)
- parent_->Register(this);
- }
BoolProperty(PropRegistry* reg, const char* name, GesturesPropBool val,
- PropertyDelegate* delegate)
+ PropertyDelegate* delegate = NULL)
: Property(reg, name, delegate), val_(val) {
if (parent_)
parent_->Register(this);
@@ -111,13 +118,7 @@ class BoolProperty : public Property {
class BoolArrayProperty : public Property {
public:
BoolArrayProperty(PropRegistry* reg, const char* name, GesturesPropBool* vals,
- size_t count)
- : Property(reg, name), vals_(vals), count_(count) {
- if (parent_)
- parent_->Register(this);
- }
- BoolArrayProperty(PropRegistry* reg, const char* name, GesturesPropBool* vals,
- size_t count, PropertyDelegate* delegate)
+ size_t count, PropertyDelegate* delegate = NULL)
: Property(reg, name, delegate), vals_(vals), count_(count) {
if (parent_)
parent_->Register(this);
@@ -133,13 +134,8 @@ class BoolArrayProperty : public Property {
class DoubleProperty : public Property {
public:
- DoubleProperty(PropRegistry* reg, const char* name, double val)
- : Property(reg, name), val_(val) {
- if (parent_)
- parent_->Register(this);
- }
DoubleProperty(PropRegistry* reg, const char* name, double val,
- PropertyDelegate* delegate)
+ PropertyDelegate* delegate = NULL)
: Property(reg, name, delegate), val_(val) {
if (parent_)
parent_->Register(this);
@@ -155,13 +151,7 @@ class DoubleProperty : public Property {
class DoubleArrayProperty : public Property {
public:
DoubleArrayProperty(PropRegistry* reg, const char* name, double* vals,
- size_t count)
- : Property(reg, name), vals_(vals), count_(count) {
- if (parent_)
- parent_->Register(this);
- }
- DoubleArrayProperty(PropRegistry* reg, const char* name, double* vals,
- size_t count, PropertyDelegate* delegate)
+ size_t count, PropertyDelegate* delegate = NULL)
: Property(reg, name, delegate), vals_(vals), count_(count) {
if (parent_)
parent_->Register(this);
@@ -177,13 +167,8 @@ class DoubleArrayProperty : public Property {
class IntProperty : public Property {
public:
- IntProperty(PropRegistry* reg, const char* name, int val)
- : Property(reg, name), val_(val) {
- if (parent_)
- parent_->Register(this);
- }
IntProperty(PropRegistry* reg, const char* name, int val,
- PropertyDelegate* delegate)
+ PropertyDelegate* delegate = NULL)
: Property(reg, name, delegate), val_(val) {
if (parent_)
parent_->Register(this);
@@ -198,13 +183,8 @@ class IntProperty : public Property {
class IntArrayProperty : public Property {
public:
- IntArrayProperty(PropRegistry* reg, const char* name, int* vals, size_t count)
- : Property(reg, name), vals_(vals), count_(count) {
- if (parent_)
- parent_->Register(this);
- }
IntArrayProperty(PropRegistry* reg, const char* name, int* vals, size_t count,
- PropertyDelegate* delegate)
+ PropertyDelegate* delegate = NULL)
: Property(reg, name, delegate), vals_(vals), count_(count) {
if (parent_)
parent_->Register(this);
@@ -220,13 +200,8 @@ class IntArrayProperty : public Property {
class StringProperty : public Property {
public:
- StringProperty(PropRegistry* reg, const char* name, const char* val)
- : Property(reg, name), val_(val) {
- if (parent_)
- parent_->Register(this);
- }
StringProperty(PropRegistry* reg, const char* name, const char* val,
- PropertyDelegate* delegate)
+ PropertyDelegate* delegate = NULL)
: Property(reg, name, delegate), val_(val) {
if (parent_)
parent_->Register(this);
diff --git a/src/gestures.cc b/src/gestures.cc
index 13aac23..475b4f5 100644
--- a/src/gestures.cc
+++ b/src/gestures.cc
@@ -470,7 +470,7 @@ void GestureInterpreter::PushHardwareState(HardwareState* hwstate) {
Log("Setting timer for %f s out.", timeout);
}
} else {
- Err("No timer!");
+ ErrOnce("No timer provider has been set, so some features won't work.");
}
}
diff --git a/src/iir_filter_interpreter.cc b/src/iir_filter_interpreter.cc
index 3b9d4a0..0558ef6 100644
--- a/src/iir_filter_interpreter.cc
+++ b/src/iir_filter_interpreter.cc
@@ -33,14 +33,21 @@ IirFilterInterpreter::IirFilterInterpreter(PropRegistry* prop_reg,
Tracer* tracer)
: FilterInterpreter(NULL, next, tracer, false),
using_iir_(true),
- b0_(prop_reg, "IIR b0", 0.0674552738890719, this),
- b1_(prop_reg, "IIR b1", 0.134910547778144, this),
- b2_(prop_reg, "IIR b2", 0.0674552738890719, this),
- b3_(prop_reg, "IIR b3", 0.0, this),
- a1_(prop_reg, "IIR a1", -1.1429805025399, this),
- a2_(prop_reg, "IIR a2", 0.412801598096189, this),
- iir_dist_thresh_(prop_reg, "IIR Distance Threshold", 10, this),
+ b0_(prop_reg, "IIR b0", 0.0674552738890719),
+ b1_(prop_reg, "IIR b1", 0.134910547778144),
+ b2_(prop_reg, "IIR b2", 0.0674552738890719),
+ b3_(prop_reg, "IIR b3", 0.0),
+ a1_(prop_reg, "IIR a1", -1.1429805025399),
+ a2_(prop_reg, "IIR a2", 0.412801598096189),
+ iir_dist_thresh_(prop_reg, "IIR Distance Threshold", 10),
adjust_iir_on_warp_(prop_reg, "Adjust IIR History On Warp", false) {
+ b0_.SetDelegate(this);
+ b1_.SetDelegate(this);
+ b2_.SetDelegate(this);
+ b3_.SetDelegate(this);
+ a1_.SetDelegate(this);
+ a2_.SetDelegate(this);
+ iir_dist_thresh_.SetDelegate(this);
InitName();
}
diff --git a/src/prop_registry.cc b/src/prop_registry.cc
index 2d9d454..58e3a1a 100644
--- a/src/prop_registry.cc
+++ b/src/prop_registry.cc
@@ -70,6 +70,18 @@ void Property::DestroyProp() {
gprop_ = NULL;
}
+void Property::SetDelegate(PropertyDelegate* delegate) {
+ bool delegate_was_late_set = !delegate_ && delegate;
+ delegate_ = delegate;
+ // In the normal path of creation of a property with a parent, the
+ // delegate is set late and the CreatePropImpl will not call the
+ // WasWritten method. So catch the transition from NULL to not
+ // NULL in this condition and make sure to call the initial
+ // WasWritten to mimick the original code.
+ if (delegate_was_late_set && parent_ && parent_->PropProvider())
+ HandleGesturesPropWritten();
+}
+
void BoolProperty::CreatePropImpl() {
GesturesPropBool orig_val = val_;
gprop_ = parent_->PropProvider()->create_bool_fn(
diff --git a/src/prop_registry_unittest.cc b/src/prop_registry_unittest.cc
index 1b994fb..23ba9d8 100644
--- a/src/prop_registry_unittest.cc
+++ b/src/prop_registry_unittest.cc
@@ -129,7 +129,7 @@ TEST(PropRegistryTest, PropChangeTest) {
ActivityLog log(&reg);
reg.set_activity_log(&log);
- DoubleProperty dp(&reg, "hi", 1234.0, NULL);
+ DoubleProperty dp(&reg, "hi", 1234.0);
EXPECT_EQ(0, log.size());
dp.HandleGesturesPropWritten();
EXPECT_EQ(1, log.size());
@@ -203,6 +203,11 @@ TEST(PropRegistryTest, SetAtCreateShouldNotifyTest) {
EXPECT_EQ(0, delegate.call_cnt_);
reg.SetPropProvider(&mock_gestures_props_provider, NULL);
EXPECT_EQ(4, delegate.call_cnt_);
+
+ BoolProperty my_bool2(&reg, "MyBool2", 1);
+ EXPECT_EQ(4, delegate.call_cnt_);
+ my_bool2.SetDelegate(&delegate);
+ EXPECT_EQ(5, delegate.call_cnt_);
}
TEST(PropRegistryTest, DoublePromoteIntTest) {